From 324b7cf75d786613bb50ea376eef7591acbd85ab Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 29 Jul 2020 14:29:10 -0400 Subject: [PATCH 1/4] Allows nanosecond resolution in search_after (#60328) This fixes `search_after` to properly parse string formatted dates that have nanosecond resolution. Closes #52424 --- .../test/search/90_search_after.yml | 132 +++++++++++++++++- .../plain/SortedNumericIndexFieldData.java | 4 +- .../index/mapper/DateFieldMapper.java | 1 + .../elasticsearch/search/DocValueFormat.java | 5 + .../search/sort/FieldSortBuilder.java | 10 +- .../search/sort/FieldSortBuilderTests.java | 2 +- 6 files changed, 146 insertions(+), 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 index aa283cda4c27f..5220980ffd0b7 100644 --- 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,130 @@ setup: - match: {hits.total: 3} - length: {hits.hits: 0 } + +--- +"date": + - 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 + 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.828" } + - match: {hits.hits.0.sort: [1571646604828] } + + # search_after with the sort + - do: + search: + index: test + body: + size: 1 + sort: [{ timestamp: desc }] + search_after: [1571646604828] + - 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.828" } + - match: {hits.hits.0.sort: [1571617804828] } + + # search_after with the formatted date + - do: + search: + index: test + body: + size: 1 + sort: [{ timestamp: desc }] + search_after: ["2019-10-21 08:30:04.828"] + - 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.828" } + - match: {hits.hits.0.sort: [1571617804828] } + +--- +"date_nanos": + - skip: + version: " - 7.99.99" + reason: fixed in 8.0.0 to be backported to 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/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericIndexFieldData.java index 84df5be537f0a..82549c0527777 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericIndexFieldData.java @@ -105,7 +105,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)); } @@ -115,7 +115,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 index 6ba9e1fb94fdf..db11c044eed49 100644 --- 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 index a98f89ba4c000..4078d5f664c0c 100644 --- 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 index 47e2813ac084d..eae89eea3418b 100644 --- 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 index a5e8eb8fa9c89..b1bcbcad62973 100644 --- 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 Date: Wed, 29 Jul 2020 15:34:40 -0400 Subject: [PATCH 2/4] Update skip --- .../rest-api-spec/test/search/90_search_after.yml | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 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 index 5220980ffd0b7..74d293d939097 100644 --- 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 @@ -119,10 +119,11 @@ - do: search: index: test + rest_total_hits_as_int: true body: size: 1 sort: [{ timestamp: desc }] - - match: {hits.total.value: 2 } + - 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" } @@ -132,11 +133,12 @@ - do: search: index: test + rest_total_hits_as_int: true body: size: 1 sort: [{ timestamp: desc }] search_after: [1571646604828] - - match: {hits.total.value: 2 } + - 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" } @@ -146,11 +148,12 @@ - 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.value: 2 } + - 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" } @@ -159,8 +162,8 @@ --- "date_nanos": - skip: - version: " - 7.99.99" - reason: fixed in 8.0.0 to be backported to 7.10.0 + version: " - 7.9.99" + reason: fixed in 7.10.0 - do: indices.create: From 1212ae6809c939e81781e959d44b13dba6d52c26 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 29 Jul 2020 15:53:51 -0400 Subject: [PATCH 3/4] Fix date format --- .../resources/rest-api-spec/test/search/90_search_after.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 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 index 74d293d939097..e9362d2c3166b 100644 --- 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 @@ -105,7 +105,7 @@ properties: timestamp: type: date - format: yyyy-MM-dd HH:mm:ss.SSS + format: 8yyyy-MM-dd HH:mm:ss.SSS - do: bulk: refresh: true @@ -173,7 +173,7 @@ properties: timestamp: type: date_nanos - format: yyyy-MM-dd HH:mm:ss.SSSSSS + format: 8yyyy-MM-dd HH:mm:ss.SSSSSS - do: bulk: refresh: true From e9a62926ce1888676dedf0be2b8d3624f62162d4 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 29 Jul 2020 15:57:58 -0400 Subject: [PATCH 4/4] Limit bwc --- .../rest-api-spec/test/search/90_search_after.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 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 index e9362d2c3166b..9f0273fbc0213 100644 --- 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 @@ -97,6 +97,10 @@ --- "date": + - skip: + version: " - 7.9.99" + reason: fixed in 7.10.0 + - do: indices.create: index: test @@ -105,7 +109,7 @@ properties: timestamp: type: date - format: 8yyyy-MM-dd HH:mm:ss.SSS + format: yyyy-MM-dd HH:mm:ss.SSS - do: bulk: refresh: true @@ -173,7 +177,7 @@ properties: timestamp: type: date_nanos - format: 8yyyy-MM-dd HH:mm:ss.SSSSSS + format: yyyy-MM-dd HH:mm:ss.SSSSSS - do: bulk: refresh: true