From 9eb9737f0993d84243a8bf76797f61fbc34f9c26 Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Wed, 29 Nov 2023 14:51:41 -0500 Subject: [PATCH 1/3] Allow mismatched sort-by field types if there are no docs to sort --- .../elasticsearch/search/sort/FieldSortIT.java | 17 ++++++++++++++++- .../action/search/SearchPhaseController.java | 13 +++++++++---- 2 files changed, 25 insertions(+), 5 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 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..f8cf156497516 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java @@ -241,7 +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); + checkSameSortTypes(results); final Sort sort = new Sort(firstTopDocs.fields); final TopFieldDocs[] shardTopDocs = results.toArray(new TopFieldDocs[numShards]); mergedTopDocs = TopDocs.merge(sort, from, topN, shardTopDocs); @@ -252,16 +252,21 @@ static TopDocs mergeTopDocs(Collection results, int topN, int from) { return mergedTopDocs; } - private static void checkSameSortTypes(Collection results, SortField[] firstSortFields) { + private static void checkSameSortTypes(Collection results) { if (results.size() < 2) return; - 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) { + 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; From f63ba18db4d00808001d07095aa14930ebd5c863 Mon Sep 17 00:00:00 2001 From: Benjamin Trent Date: Wed, 29 Nov 2023 14:53:51 -0500 Subject: [PATCH 2/3] Update docs/changelog/102779.yaml --- docs/changelog/102779.yaml | 5 +++++ 1 file changed, 5 insertions(+) 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: [] From 4253403dc912055669583780b391e25a65eefded Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Thu, 30 Nov 2023 12:08:58 -0500 Subject: [PATCH 3/3] Fixing failing test case --- .../action/search/SearchPhaseController.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) 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 f8cf156497516..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); - 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,8 +251,9 @@ static TopDocs mergeTopDocs(Collection results, int topN, int from) { return mergedTopDocs; } - private static void checkSameSortTypes(Collection results) { - 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 = null; boolean isFirstResult = true; @@ -264,12 +264,13 @@ private static void checkSameSortTypes(Collection results) { } 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(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; @@ -279,7 +280,7 @@ private static void checkSameSortTypes(Collection results) { 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 [" @@ -294,6 +295,7 @@ private static void checkSameSortTypes(Collection results) { } } } + return sort; } private static SortField.Type getType(SortField sortField) {