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
2020-10-06 06:37:31 -04:00 · 2020-10-06 06:37:31 -04:00 · bea0ead08a
parent 8f4ef40f78
commit bea0ead08a
6 changed files with 178 additions and 9 deletions
--- 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
--- 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<Field> 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);
        }
--- 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
--- 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() {
--- 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" }
--- 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"] }