Skip to content

Commit

Permalink
Fix sorted query when date_nanos is used as the numeric_type (#64183)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@jimczi
jimczi committed Oct 27, 2020
1 parent 89e3a31 commit e34014e
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 23 deletions.
76 changes: 60 additions & 16 deletions server/src/internalClusterTest/java/org/elasticsearch/search/sort/FieldSortIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -1794,38 +1818,58 @@ 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]);
}

{
builders.clear();
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"));
}

{
builders.clear();
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"));
}
}

Expand Down
6 changes: 1 addition & 5 deletions server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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.");
Expand Down

0 comments on commit e34014e

Please sign in to comment.