From bea0ead08aa81f1a5ff8aa7c97c40259aaf315c7 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Tue, 6 Oct 2020 06:37:31 -0400 Subject: [PATCH] Fix fields retrieval on unsinged_long field (#63310) This fixes fields retrieval on unsigned_long field 1) For docvalue_fields a custom UnsignedLongLeafFieldData::getLeafValueFetcher is implemented that correctly retrieves doc values. 2) For stored fields, an error was fixed in UnsignedLongFieldMapper how stored values were stored. Before they were incorrectly stored in the shifted format, now they are stored as original values in String format. Relates to #60050 Backport for #63119 --- .../mapping/types/unsigned_long.asciidoc | 3 + .../unsignedlong/UnsignedLongFieldMapper.java | 21 ++++- .../UnsignedLongLeafFieldData.java | 23 ++++++ .../UnsignedLongFieldMapperTests.java | 2 +- .../test/unsigned_long/10_basic.yml | 56 +++++++++++++ .../test/unsigned_long/20_null_value.yml | 82 +++++++++++++++++-- 6 files changed, 178 insertions(+), 9 deletions(-) diff --git a/docs/reference/mapping/types/unsigned_long.asciidoc b/docs/reference/mapping/types/unsigned_long.asciidoc index 0f036a00fb3..7539db57e91 100644 --- a/docs/reference/mapping/types/unsigned_long.asciidoc +++ b/docs/reference/mapping/types/unsigned_long.asciidoc @@ -103,6 +103,9 @@ Similarly to sort values, script values of an `unsigned_long` field return a `Number` representing a `Long` or `BigInteger`. The same values: `Long` or `BigInteger` are used for `terms` aggregations. +==== Stored fields +A stored field of `unsigned_long` is stored and returned as `String`. + ==== Queries with mixed numeric types Searches with mixed numeric types one of which is `unsigned_long` are diff --git a/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapper.java b/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapper.java index 748bc9ae4b2..3c53585b874 100644 --- a/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapper.java +++ b/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapper.java @@ -8,8 +8,10 @@ package org.elasticsearch.xpack.unsignedlong; import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.exc.InputCoercionException; +import org.apache.lucene.document.Field; import org.apache.lucene.document.LongPoint; import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.document.StoredField; import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.IndexSortSortedNumericDocValuesRangeQuery; import org.apache.lucene.search.MatchNoDocsQuery; @@ -25,7 +27,6 @@ import org.elasticsearch.index.mapper.FieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.mapper.MapperService; -import org.elasticsearch.index.mapper.NumberFieldMapper; import org.elasticsearch.index.mapper.ParametrizedFieldMapper; import org.elasticsearch.index.mapper.ParseContext; import org.elasticsearch.index.mapper.SimpleMappedFieldType; @@ -40,6 +41,7 @@ import java.io.IOException; import java.math.BigDecimal; import java.math.BigInteger; import java.time.ZoneId; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -454,15 +456,28 @@ public class UnsignedLongFieldMapper extends ParametrizedFieldMapper { } } } + boolean isNullValue = false; if (numericValue == null) { numericValue = nullValueIndexed; if (numericValue == null) return; + isNullValue = true; } else { numericValue = unsignedToSortableSignedLong(numericValue); } - context.doc() - .addAll(NumberFieldMapper.NumberType.LONG.createFields(fieldType().name(), numericValue, indexed, hasDocValues, stored)); + List fields = new ArrayList<>(); + if (indexed) { + fields.add(new LongPoint(fieldType().name(), numericValue)); + } + if (hasDocValues) { + fields.add(new SortedNumericDocValuesField(fieldType().name(), numericValue)); + } + if (stored) { + // for stored field, keeping original unsigned_long value in the String form + String storedValued = isNullValue ? nullValue : Long.toUnsignedString(unsignedToSortableSignedLong(numericValue)); + fields.add(new StoredField(fieldType().name(), storedValued)); + } + context.doc().addAll(fields); if (hasDocValues == false && (stored || indexed)) { createFieldNamesField(context); } diff --git a/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongLeafFieldData.java b/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongLeafFieldData.java index 8a872e0f915..001506bbb7d 100644 --- a/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongLeafFieldData.java +++ b/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongLeafFieldData.java @@ -15,6 +15,8 @@ import org.elasticsearch.index.fielddata.NumericDoubleValues; import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.fielddata.SortedBinaryDocValues; import org.elasticsearch.index.fielddata.SortedNumericDoubleValues; +import org.elasticsearch.index.mapper.DocValueFetcher; +import org.elasticsearch.search.DocValueFormat; import java.io.IOException; @@ -89,6 +91,27 @@ public class UnsignedLongLeafFieldData implements LeafNumericFieldData { signedLongFD.close(); } + @Override + public DocValueFetcher.Leaf getLeafValueFetcher(DocValueFormat format) { + SortedNumericDocValues values = getLongValues(); + return new DocValueFetcher.Leaf() { + @Override + public boolean advanceExact(int docId) throws IOException { + return values.advanceExact(docId); + } + + @Override + public int docValueCount() { + return values.docValueCount(); + } + + @Override + public Object nextValue() throws IOException { + return format.format(values.nextValue()); + } + }; + } + private static double convertUnsignedLongToDouble(long value) { if (value < 0L) { return sortableSignedLongToUnsigned(value); // add 2 ^ 63 diff --git a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java index 7273ec14038..2345aef09ca 100644 --- a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java +++ b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java @@ -182,7 +182,7 @@ public class UnsignedLongFieldMapperTests extends MapperTestCase { assertEquals(9223372036854775807L, dvField.numericValue().longValue()); IndexableField storedField = fields[2]; assertTrue(storedField.fieldType().stored()); - assertEquals(9223372036854775807L, storedField.numericValue().longValue()); + assertEquals("18446744073709551615", storedField.stringValue()); } public void testCoerceMappingParameterIsIllegal() { diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/unsigned_long/10_basic.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/unsigned_long/10_basic.yml index a242f926773..53441a2cf3d 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/unsigned_long/10_basic.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/unsigned_long/10_basic.yml @@ -12,6 +12,7 @@ setup: properties: ul: type: unsigned_long + store: true - do: bulk: @@ -243,3 +244,58 @@ setup: - match: { aggregations.ul_range.buckets.0.doc_count: 1 } - match: { aggregations.ul_range.buckets.1.doc_count: 2 } - match: { aggregations.ul_range.buckets.2.doc_count: 2 } + + +--- +"Fields retrieval": + + # fields API + - do: + search: + index: test1 + body: + query: + match_all: {} + fields : [ "ul" ] + sort: [ { ul: asc } ] + _source : false + + - match: { hits.hits.0.fields.ul.0 : 0 } + - match: { hits.hits.1.fields.ul.0 : 9223372036854775807 } + - match: { hits.hits.2.fields.ul.0 : 9223372036854775808 } + - match: { hits.hits.3.fields.ul.0 : 18446744073709551614 } + - match: { hits.hits.4.fields.ul.0 : 18446744073709551615 } + + # doc values + - do: + search: + index: test1 + body: + query: + match_all: {} + docvalue_fields: [ "ul" ] + sort: [ { ul: asc } ] + _source : false + + - match: { hits.hits.0.fields.ul.0 : 0 } + - match: { hits.hits.1.fields.ul.0 : 9223372036854775807 } + - match: { hits.hits.2.fields.ul.0 : 9223372036854775808 } + - match: { hits.hits.3.fields.ul.0 : 18446744073709551614 } + - match: { hits.hits.4.fields.ul.0 : 18446744073709551615 } + + # stored fields + - do: + search: + index: test1 + body: + query: + match_all: {} + stored_fields: [ "ul" ] + sort: [ { ul: asc } ] + _source : false + + - match: { hits.hits.0.fields.ul.0 : "0" } + - match: { hits.hits.1.fields.ul.0 : "9223372036854775807" } + - match: { hits.hits.2.fields.ul.0 : "9223372036854775808" } + - match: { hits.hits.3.fields.ul.0 : "18446744073709551614" } + - match: { hits.hits.4.fields.ul.0 : "18446744073709551615" } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/unsigned_long/20_null_value.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/unsigned_long/20_null_value.yml index c07e02e508e..9b2c4a62474 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/unsigned_long/20_null_value.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/unsigned_long/20_null_value.yml @@ -1,5 +1,4 @@ ---- -"Null value": +setup: - skip: version: " - 7.9.99" reason: "unsigned_long was added in 7.10" @@ -13,6 +12,7 @@ ul: type: unsigned_long null_value: 17446744073709551615 + store: true - do: bulk: @@ -30,7 +30,8 @@ { "index": {"_id" : "5_missing"} } {} - # term query +--- +"Term query" : - do: search: index: test1 @@ -43,7 +44,8 @@ - match: {hits.hits.1._id: "3_null" } - # asc sort +--- +"Asc sort" : - do: search: index: test1 @@ -61,7 +63,8 @@ - match: {hits.hits.4._id: "5_missing" } - match: {hits.hits.4.sort: [18446744073709551615] } - # desc sort +--- +"Desc sort" : - do: search: index: test1 @@ -78,3 +81,72 @@ - match: {hits.hits.3.sort: [17446744073709551615] } - match: {hits.hits.4._id: "1" } - match: {hits.hits.4.sort: [0] } + +--- +"Fields retrieval" : + + # fields API + - do: + search: + index: test1 + body: + query: + match_all: {} + fields: [ "ul" ] + sort: { ul: { order: desc, missing: "_first" } } + _source: false + + - match: { hits.hits.0._id: "5_missing" } + - is_false: hits.hits.0.fields + - match: { hits.hits.1._id: "4" } + - match: { hits.hits.1.fields.ul: [18446744073709551614] } + - match: { hits.hits.2._id: "2_null" } + - match: { hits.hits.2.fields.ul: [17446744073709551615] } + - match: { hits.hits.3._id: "3_null" } + - match: { hits.hits.3.fields.ul: [17446744073709551615] } + - match: { hits.hits.4._id: "1" } + - match: { hits.hits.4.fields.ul: [0] } + + # doc values fields + - do: + search: + index: test1 + body: + query: + match_all: {} + docvalue_fields: [ "ul" ] + sort: { ul: { order: desc, missing: "_first" } } + _source: false + + - match: { hits.hits.0._id: "5_missing" } + - is_false: hits.hits.0.fields + - match: { hits.hits.1._id: "4" } + - match: { hits.hits.1.fields.ul: [18446744073709551614] } + - match: { hits.hits.2._id: "2_null" } + - match: { hits.hits.2.fields.ul: [17446744073709551615] } + - match: { hits.hits.3._id: "3_null" } + - match: { hits.hits.3.fields.ul: [17446744073709551615] } + - match: { hits.hits.4._id: "1" } + - match: { hits.hits.4.fields.ul: [0] } + + # stored fields + - do: + search: + index: test1 + body: + query: + match_all: {} + stored_fields: [ "ul" ] + sort: { ul: { order: desc, missing: "_first" } } + _source: false + + - match: { hits.hits.0._id : "5_missing" } + - is_false: hits.hits.0.fields + - match: { hits.hits.1._id : "4" } + - match: { hits.hits.1.fields.ul : ["18446744073709551614"] } + - match: { hits.hits.2._id : "2_null" } + - match: { hits.hits.2.fields.ul : ["17446744073709551615"] } + - match: { hits.hits.3._id : "3_null" } + - match: { hits.hits.3.fields.ul : ["17446744073709551615"] } + - match: { hits.hits.4._id : "1" } + - match: { hits.hits.4.fields.ul : ["0"] }