Allows nanosecond resolution in search_after (backport of #60328) (#6…

…0426) Allows nanosecond resolution in search_after (#60328) This fixes `search_after` to properly parse string formatted dates that have nanosecond resolution. Closes #52424
elastic · Aug 3, 2020 · 2cde43b · 2cde43b
1 parent b0d601f
commit 2cde43b
Show file tree

Hide file tree

Showing 6 changed files with 153 additions and 8 deletions.
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml
@@ -1,4 +1,4 @@
-setup:
+"search with search_after parameter":
   - do:
       indices.create:
           index:  test
@@ -24,9 +24,6 @@ setup:
       indices.refresh:
         index: test
 
----
-"search with search_after parameter":
-
   - do:
       search:
         rest_total_hits_as_int: true
@@ -97,3 +94,137 @@ setup:
 
   - match: {hits.total: 3}
   - length: {hits.hits: 0 }
+
+---
+"date":
+  - skip:
+      version: " - 7.9.99"
+      reason: fixed in 7.10.0
+
+  - do:
+      indices.create:
+        index: test
+        body:
+          mappings:
+            properties:
+              timestamp:
+                type: date
+                format: yyyy-MM-dd HH:mm:ss.SSS
+  - do:
+      bulk:
+        refresh: true
+        index: test
+        body: |
+          {"index":{}}
+          {"timestamp":"2019-10-21 00:30:04.828"}
+          {"index":{}}
+          {"timestamp":"2019-10-21 08:30:04.828"}
+
+  - do:
+      search:
+        index: test
+        rest_total_hits_as_int: true
+        body:
+          size: 1
+          sort: [{ timestamp: desc }]
+  - match: {hits.total: 2 }
+  - length: {hits.hits: 1 }
+  - match: {hits.hits.0._index: test }
+  - match: {hits.hits.0._source.timestamp: "2019-10-21 08:30:04.828" }
+  - match: {hits.hits.0.sort: [1571646604828] }
+
+  # search_after with the sort
+  - do:
+      search:
+        index: test
+        rest_total_hits_as_int: true
+        body:
+          size: 1
+          sort: [{ timestamp: desc }]
+          search_after: [1571646604828]
+  - match: {hits.total: 2 }
+  - length: {hits.hits: 1 }
+  - match: {hits.hits.0._index: test }
+  - match: {hits.hits.0._source.timestamp: "2019-10-21 00:30:04.828" }
+  - match: {hits.hits.0.sort: [1571617804828] }
+
+  # search_after with the formatted date
+  - do:
+      search:
+        index: test
+        rest_total_hits_as_int: true
+        body:
+          size: 1
+          sort: [{ timestamp: desc }]
+          search_after: ["2019-10-21 08:30:04.828"]
+  - match: {hits.total: 2 }
+  - length: {hits.hits: 1 }
+  - match: {hits.hits.0._index: test }
+  - match: {hits.hits.0._source.timestamp: "2019-10-21 00:30:04.828" }
+  - match: {hits.hits.0.sort: [1571617804828] }
+
+---
+"date_nanos":
+  - skip:
+      version: " - 7.9.99"
+      reason: fixed in 7.10.0
+
+  - do:
+      indices.create:
+        index: test
+        body:
+          mappings:
+            properties:
+              timestamp:
+                type: date_nanos
+                format: yyyy-MM-dd HH:mm:ss.SSSSSS
+  - do:
+      bulk:
+        refresh: true
+        index: test
+        body: |
+          {"index":{}}
+          {"timestamp":"2019-10-21 00:30:04.828740"}
+          {"index":{}}
+          {"timestamp":"2019-10-21 08:30:04.828733"}
+
+  - do:
+      search:
+        index: test
+        body:
+          size: 1
+          sort: [{ timestamp: desc }]
+  - match: {hits.total.value: 2 }
+  - length: {hits.hits: 1 }
+  - match: {hits.hits.0._index: test }
+  - match: {hits.hits.0._source.timestamp: "2019-10-21 08:30:04.828733" }
+  - match: {hits.hits.0.sort: [1571646604828733000] }
+
+  # search_after with the sort
+  - do:
+      search:
+        index: test
+        body:
+          size: 1
+          sort: [{ timestamp: desc }]
+          search_after: [1571646604828733000]
+  - match: {hits.total.value: 2 }
+  - length: {hits.hits: 1 }
+  - match: {hits.hits.0._index: test }
+  - match: {hits.hits.0._source.timestamp: "2019-10-21 00:30:04.828740" }
+  - match: {hits.hits.0.sort: [1571617804828740000] }
+
+  # search_after with the formatted date
+  - do:
+      search:
+        index: test
+        body:
+          size: 1
+          sort: [{ timestamp: desc }]
+          search_after: ["2019-10-21 08:30:04.828733"]
+  - match: {hits.total.value: 2 }
+  - length: {hits.hits: 1 }
+  - match: {hits.hits.0._index: test }
+  - match: {hits.hits.0._source.timestamp: "2019-10-21 00:30:04.828740" }
+  - match: {hits.hits.0.sort: [1571617804828740000] }
+
diff --git a/...er/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericIndexFieldData.java b/...er/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericIndexFieldData.java
@@ -100,7 +100,7 @@ protected boolean sortRequiresCustomComparator() {
     @Override
     protected XFieldComparatorSource dateComparatorSource(Object missingValue, MultiValueMode sortMode, Nested nested) {
         if (numericType == NumericType.DATE_NANOSECONDS) {
-            // converts date values to nanosecond resolution
+            // converts date_nanos values to millisecond resolution
             return new LongValuesComparatorSource(this, missingValue,
                 sortMode, nested, dvs -> convertNumeric(dvs, DateUtils::toMilliSeconds));
         }
@@ -110,7 +110,7 @@ protected XFieldComparatorSource dateComparatorSource(Object missingValue, Multi
     @Override
     protected XFieldComparatorSource dateNanosComparatorSource(Object missingValue, MultiValueMode sortMode, Nested nested) {
         if (numericType == NumericType.DATE) {
-            // converts date_nanos values to millisecond resolution
+            // converts date values to nanosecond resolution
             return new LongValuesComparatorSource(this, missingValue,
                 sortMode, nested, dvs -> convertNumeric(dvs, DateUtils::toNanoSeconds));
         }

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java
@@ -451,6 +451,7 @@ public DocValueFormat docValueFormat(@Nullable String format, ZoneId timeZone) {
             }
             // the resolution here is always set to milliseconds, as aggregations use this formatter mainly and those are always in
             // milliseconds. The only special case here is docvalue fields, which are handled somewhere else
+            // TODO maybe aggs should force millis because lots so of other places want nanos?
             return new DocValueFormat.DateTime(dateTimeFormatter, timeZone, Resolution.MILLISECONDS);
         }
     }

diff --git a/server/src/main/java/org/elasticsearch/search/DocValueFormat.java b/server/src/main/java/org/elasticsearch/search/DocValueFormat.java
@@ -274,6 +274,11 @@ public long parseLong(String value, boolean roundUp, LongSupplier now) {
         public double parseDouble(String value, boolean roundUp, LongSupplier now) {
             return parseLong(value, roundUp, now);
         }
+
+        @Override
+        public String toString() {
+            return "DocValueFormat.DateTime(" + formatter + ", " + timeZone + ", " + resolution + ")";
+        }
     }
 
     DocValueFormat GEOHASH = new DocValueFormat() {

diff --git a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java
@@ -404,6 +404,7 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException {
             throw new QueryShardException(context, "we only support AVG, MEDIAN and SUM on number based fields");
         }
         final SortField field;
+        boolean isNanosecond = false;
         if (numericType != null) {
             if (fieldData instanceof IndexNumericFieldData == false) {
                 throw new QueryShardException(context,
@@ -414,8 +415,15 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException {
             field = numericFieldData.sortField(resolvedType, missing, localSortMode(), nested, reverse);
         } else {
             field = fieldData.sortField(missing, localSortMode(), nested, reverse);
+            if (fieldData instanceof IndexNumericFieldData) {
+                isNanosecond = ((IndexNumericFieldData) fieldData).getNumericType() == NumericType.DATE_NANOSECONDS;
+            }
+        }
+        DocValueFormat format = fieldType.docValueFormat(null, null);
+        if (isNanosecond) {
+            format = DocValueFormat.withNanosecondResolution(format);
         }
-        return new SortFieldAndFormat(field, fieldType.docValueFormat(null, null));
+        return new SortFieldAndFormat(field, format);
     }
 
     public boolean canRewriteToMatchNone() {

diff --git a/server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java b/server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java
@@ -80,7 +80,7 @@ public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder
     /**
      * {@link #provideMappedFieldType(String)} will return a
      */
-    private static String MAPPED_STRING_FIELDNAME = "_stringField";
+    private static final String MAPPED_STRING_FIELDNAME = "_stringField";
 
     @Override
     protected FieldSortBuilder createTestItem() {