From 43dc74bfaf9ab8565309616b94c598b4accc8362 Mon Sep 17 00:00:00 2001 From: Benjamin Trent Date: Thu, 30 Nov 2023 14:39:41 -0500 Subject: [PATCH] Allow mismatched sort-by field types if there are no docs to sort (#102779) When searching multiple indices and a field only exists in ONE of the indices, we should allow sorting by that field, regardless of the "unmapped" type provided. closes: https://github.com/elastic/elasticsearch/issues/102723 --- docs/changelog/102779.yaml | 5 ++++ .../search/sort/FieldSortIT.java | 17 +++++++++++++- .../action/search/SearchPhaseController.java | 23 ++++++++++++------- 3 files changed, 36 insertions(+), 9 deletions(-) create mode 100644 docs/changelog/102779.yaml diff --git a/docs/changelog/102779.yaml b/docs/changelog/102779.yaml new file mode 100644 index 0000000000000..7bbecb29665bd --- /dev/null +++ b/docs/changelog/102779.yaml @@ -0,0 +1,5 @@ +pr: 102779 +summary: Allow mismatched sort-by field types if there are no docs to sort +area: Search +type: bug +issues: [] 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 2cd68398e211f..21a784c16c8e9 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/sort/FieldSortIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/sort/FieldSortIT.java @@ -150,7 +150,7 @@ public void testIssue8226() { ); } - public void testIssue6614() throws ExecutionException, InterruptedException { + public void testIssue6614() throws InterruptedException { List builders = new ArrayList<>(); boolean strictTimeBasedIndices = randomBoolean(); final int numIndices = randomIntBetween(2, 25); // at most 25 days in the month @@ -2137,4 +2137,19 @@ public void testSortMixedFieldTypes() { } } + public void testSortMixedFieldTypesWithNoDocsForOneType() { + assertAcked(prepareCreate("index_long").setMapping("foo", "type=long").get()); + assertAcked(prepareCreate("index_other").setMapping("bar", "type=keyword").get()); + assertAcked(prepareCreate("index_double").setMapping("foo", "type=double").get()); + + prepareIndex("index_long").setId("1").setSource("foo", "123").get(); + prepareIndex("index_long").setId("2").setSource("foo", "124").get(); + prepareIndex("index_other").setId("1").setSource("bar", "124").get(); + refresh(); + + assertNoFailures( + prepareSearch("index_long", "index_double", "index_other").addSort(new FieldSortBuilder("foo").unmappedType("boolean")) + .setSize(10) + ); + } } diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java b/server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java index 0662e94b519d9..e262003935969 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java @@ -241,8 +241,7 @@ static TopDocs mergeTopDocs(Collection results, int topN, int from) { final TopFieldGroups[] shardTopDocs = results.toArray(new TopFieldGroups[numShards]); mergedTopDocs = TopFieldGroups.merge(sort, from, topN, shardTopDocs, false); } else if (topDocs instanceof TopFieldDocs firstTopDocs) { - checkSameSortTypes(results, firstTopDocs.fields); - final Sort sort = new Sort(firstTopDocs.fields); + final Sort sort = checkSameSortTypes(results, firstTopDocs.fields); final TopFieldDocs[] shardTopDocs = results.toArray(new TopFieldDocs[numShards]); mergedTopDocs = TopDocs.merge(sort, from, topN, shardTopDocs); } else { @@ -252,19 +251,26 @@ static TopDocs mergeTopDocs(Collection results, int topN, int from) { return mergedTopDocs; } - private static void checkSameSortTypes(Collection results, SortField[] firstSortFields) { - if (results.size() < 2) return; + private static Sort checkSameSortTypes(Collection results, SortField[] firstSortFields) { + Sort sort = new Sort(firstSortFields); + if (results.size() < 2) return sort; - SortField.Type[] firstTypes = new SortField.Type[firstSortFields.length]; + SortField.Type[] firstTypes = null; boolean isFirstResult = true; for (TopDocs topDocs : results) { + // We don't actually merge in empty score docs, so ignore potentially mismatched types if there are no docs + if (topDocs.scoreDocs == null || topDocs.scoreDocs.length == 0) { + continue; + } SortField[] curSortFields = ((TopFieldDocs) topDocs).fields; if (isFirstResult) { + sort = new Sort(curSortFields); + firstTypes = new SortField.Type[curSortFields.length]; for (int i = 0; i < curSortFields.length; i++) { - firstTypes[i] = getType(firstSortFields[i]); + firstTypes[i] = getType(curSortFields[i]); if (firstTypes[i] == SortField.Type.CUSTOM) { // for custom types that we can't resolve, we can't do the check - return; + return sort; } } isFirstResult = false; @@ -274,7 +280,7 @@ private static void checkSameSortTypes(Collection results, SortField[] if (curType != firstTypes[i]) { if (curType == SortField.Type.CUSTOM) { // for custom types that we can't resolve, we can't do the check - return; + return sort; } throw new IllegalArgumentException( "Can't sort on field [" @@ -289,6 +295,7 @@ private static void checkSameSortTypes(Collection results, SortField[] } } } + return sort; } private static SortField.Type getType(SortField sortField) {