From e34014eb6a9a166cbe38c2d2806161e279f3618d Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Tue, 27 Oct 2020 10:31:54 +0100 Subject: [PATCH] Fix sorted query when date_nanos is used as the numeric_type (#64183) The formatting of the global bottom value does not take the resolution of the provided numeric_type into account. This change fixes this bug by providing the resolution directly in the doc value format if the numeric_type is provided as `date_nanos`. Closes #63719 --- .../search/sort/FieldSortIT.java | 76 +++++++++++++++---- .../search/sort/FieldSortBuilder.java | 6 +- .../search/sort/FieldSortBuilderTests.java | 4 +- 3 files changed, 63 insertions(+), 23 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/sort/FieldSortIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/sort/FieldSortIT.java index da95bcc4b4561..92aa113e25eda 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/sort/FieldSortIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/sort/FieldSortIT.java @@ -1768,7 +1768,7 @@ public void testCastDate() throws Exception { { SearchResponse response = client().prepareSearch() .setQuery(matchAllQuery()) - .setSize(builders.size()) + .setSize(2) .addSort(SortBuilders.fieldSort("field").setNumericType("date")) .get(); SearchHits hits = response.getHits(); @@ -1779,12 +1779,36 @@ public void testCastDate() throws Exception { } assertEquals(1712879236854L, hits.getAt(0).getSortValues()[0]); assertEquals(1712879237000L, hits.getAt(1).getSortValues()[0]); + + response = client().prepareSearch() + .setMaxConcurrentShardRequests(1) + .setQuery(matchAllQuery()) + .setSize(1) + .addSort(SortBuilders.fieldSort("field").setNumericType("date")) + .get(); + hits = response.getHits(); + + assertEquals(1, hits.getHits().length); + assertThat(hits.getAt(0).getSortValues()[0].getClass(), equalTo(Long.class)); + assertEquals(1712879236854L, hits.getAt(0).getSortValues()[0]); + + response = client().prepareSearch() + .setMaxConcurrentShardRequests(1) + .setQuery(matchAllQuery()) + .setSize(1) + .addSort(SortBuilders.fieldSort("field").order(SortOrder.DESC).setNumericType("date")) + .get(); + hits = response.getHits(); + + assertEquals(1, hits.getHits().length); + assertThat(hits.getAt(0).getSortValues()[0].getClass(), equalTo(Long.class)); + assertEquals(1712879237000L, hits.getAt(0).getSortValues()[0]); } { SearchResponse response = client().prepareSearch() .setQuery(matchAllQuery()) - .setSize(builders.size()) + .setSize(2) .addSort(SortBuilders.fieldSort("field").setNumericType("date_nanos")) .get(); SearchHits hits = response.getHits(); @@ -1794,6 +1818,28 @@ public void testCastDate() throws Exception { } assertEquals(1712879236854775807L, hits.getAt(0).getSortValues()[0]); assertEquals(1712879237000000000L, hits.getAt(1).getSortValues()[0]); + + response = client().prepareSearch() + .setMaxConcurrentShardRequests(1) + .setQuery(matchAllQuery()) + .setSize(1) + .addSort(SortBuilders.fieldSort("field").setNumericType("date_nanos")) + .get(); + hits = response.getHits(); + assertEquals(1, hits.getHits().length); + assertThat(hits.getAt(0).getSortValues()[0].getClass(), equalTo(Long.class)); + assertEquals(1712879236854775807L, hits.getAt(0).getSortValues()[0]); + + response = client().prepareSearch() + .setMaxConcurrentShardRequests(1) + .setQuery(matchAllQuery()) + .setSize(1) + .addSort(SortBuilders.fieldSort("field").order(SortOrder.DESC).setNumericType("date_nanos")) + .get(); + hits = response.getHits(); + assertEquals(1, hits.getHits().length); + assertThat(hits.getAt(0).getSortValues()[0].getClass(), equalTo(Long.class)); + assertEquals(1712879237000000000L, hits.getAt(0).getSortValues()[0]); } { @@ -1801,15 +1847,14 @@ public void testCastDate() throws Exception { builders.add(client().prepareIndex("index_date", "_doc") .setSource("field", "1905-04-11T23:47:17")); indexRandom(true, true, builders); - SearchPhaseExecutionException exc = expectThrows(SearchPhaseExecutionException.class, - () -> client().prepareSearch() + SearchResponse response = client().prepareSearch() .setQuery(matchAllQuery()) - .setSize(builders.size()) - .setAllowPartialSearchResults(false) + .setSize(1) .addSort(SortBuilders.fieldSort("field").setNumericType("date_nanos")) - .get() - ); - assertThat(exc.toString(), containsString("are before the epoch in 1970")); + .get(); + assertNotNull(response.getShardFailures()); + assertThat(response.getShardFailures().length, equalTo(1)); + assertThat(response.getShardFailures()[0].toString(), containsString("are before the epoch in 1970")); } { @@ -1817,15 +1862,14 @@ public void testCastDate() throws Exception { builders.add(client().prepareIndex("index_date", "_doc") .setSource("field", "2346-04-11T23:47:17")); indexRandom(true, true, builders); - SearchPhaseExecutionException exc = expectThrows(SearchPhaseExecutionException.class, - () -> client().prepareSearch() + SearchResponse response = client().prepareSearch() .setQuery(QueryBuilders.rangeQuery("field").gt("1970-01-01")) - .setSize(builders.size()) - .setAllowPartialSearchResults(false) + .setSize(10) .addSort(SortBuilders.fieldSort("field").setNumericType("date_nanos")) - .get() - ); - assertThat(exc.toString(), containsString("are after 2262")); + .get(); + assertNotNull(response.getShardFailures()); + assertThat(response.getShardFailures().length, equalTo(1)); + assertThat(response.getShardFailures()[0].toString(), containsString("are after 2262")); } } 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 f0151084f5def..1151c4348a2e3 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -412,6 +412,7 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException { IndexNumericFieldData numericFieldData = (IndexNumericFieldData) fieldData; NumericType resolvedType = resolveNumericType(numericType); field = numericFieldData.sortField(resolvedType, missing, localSortMode(), nested, reverse); + isNanosecond = resolvedType == NumericType.DATE_NANOSECONDS; } else { field = fieldData.sortField(missing, localSortMode(), nested, reverse); if (fieldData instanceof IndexNumericFieldData) { @@ -452,11 +453,6 @@ public boolean isBottomSortShardDisjoint(QueryShardContext context, SearchSortVa DocValueFormat docValueFormat = bottomSortValues.getSortValueFormats()[0]; final DateMathParser dateMathParser; if (docValueFormat instanceof DocValueFormat.DateTime) { - if (fieldType instanceof DateFieldType && ((DateFieldType) fieldType).resolution() == NANOSECONDS) { - // we parse the formatted value with the resolution of the local field because - // the provided format can use a different one (date vs date_nanos). - docValueFormat = DocValueFormat.withNanosecondResolution(docValueFormat); - } dateMathParser = ((DocValueFormat.DateTime) docValueFormat).getDateMathParser(); } else { dateMathParser = null; 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 b1bcbcad62973..4c46094be287c 100644 --- a/server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java @@ -128,7 +128,7 @@ public FieldSortBuilder randomFieldSortBuilder() { } } if (randomBoolean()) { - builder.setNumericType(randomFrom(random(), "long", "double", "date", "date_nanos")); + builder.setNumericType(randomFrom(random(), "long", "double")); } return builder; } @@ -166,7 +166,7 @@ protected FieldSortBuilder mutate(FieldSortBuilder original) throws IOException break; case 5: mutated.setNumericType(randomValueOtherThan(original.getNumericType(), - () -> randomFrom("long", "double", "date", "date_nanos"))); + () -> randomFrom("long", "double"))); break; default: throw new IllegalStateException("Unsupported mutation.");