Skip to content

Commit 0555c3d

Browse files
committed
Disable sort optimization on search collapsing
Collapse search queries that sort by a field can throw an ArrayStoreException due to a bug in the [sort optimization](elastic#51852) introduced in 7.7.0. Search collapsing were not supposed to be eligible for this sort optimization so this change explicitly filters them from this new feature.
1 parent fbc8950 commit 0555c3d

File tree

2 files changed

+39
-11
lines changed

2 files changed

+39
-11
lines changed

server/src/main/java/org/elasticsearch/action/search/SearchQueryThenFetchAsyncAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@ protected void onShardResult(SearchPhaseResult result, SearchShardIterator shard
9393
if (queryResult.isNull() == false
9494
// disable sort optims for scroll requests because they keep track of the last bottom doc locally (per shard)
9595
&& getRequest().scroll() == null
96-
&& queryResult.topDocs().topDocs instanceof TopFieldDocs) {
96+
&& queryResult.topDocs() != null
97+
&& queryResult.topDocs().topDocs.getClass() == TopFieldDocs.class) {
9798
TopFieldDocs topDocs = (TopFieldDocs) queryResult.topDocs().topDocs;
9899
if (bottomSortCollector == null) {
99100
synchronized (this) {

server/src/test/java/org/elasticsearch/action/search/SearchQueryThenFetchAsyncActionTests.java

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.apache.lucene.search.SortField;
2424
import org.apache.lucene.search.TopFieldDocs;
2525
import org.apache.lucene.search.TotalHits;
26+
import org.apache.lucene.search.grouping.CollapseTopFieldDocs;
2627
import org.elasticsearch.Version;
2728
import org.elasticsearch.action.OriginalIndices;
2829
import org.elasticsearch.cluster.node.DiscoveryNode;
@@ -36,6 +37,7 @@
3637
import org.elasticsearch.search.SearchPhaseResult;
3738
import org.elasticsearch.search.SearchShardTarget;
3839
import org.elasticsearch.search.builder.SearchSourceBuilder;
40+
import org.elasticsearch.search.collapse.CollapseBuilder;
3941
import org.elasticsearch.search.internal.AliasFilter;
4042
import org.elasticsearch.search.internal.SearchContextId;
4143
import org.elasticsearch.search.internal.ShardSearchRequest;
@@ -58,14 +60,18 @@
5860

5961
public class SearchQueryThenFetchAsyncActionTests extends ESTestCase {
6062
public void testBottomFieldSort() throws Exception {
61-
testCase(false);
63+
testCase(false, false);
6264
}
6365

6466
public void testScrollDisableBottomFieldSort() throws Exception {
65-
testCase(true);
67+
testCase(true, false);
6668
}
6769

68-
private void testCase(boolean withScroll) throws Exception {
70+
public void testCollapseDisableBottomFieldSort() throws Exception {
71+
testCase(false, true);
72+
}
73+
74+
private void testCase(boolean withScroll, boolean withCollapse) throws Exception {
6975
final TransportSearchAction.SearchTimeProvider timeProvider =
7076
new TransportSearchAction.SearchTimeProvider(0, System.nanoTime(), System::nanoTime);
7177

@@ -95,12 +101,24 @@ public void sendExecuteQuery(Transport.Connection connection, ShardSearchRequest
95101
QuerySearchResult queryResult = new QuerySearchResult(new SearchContextId("N/A", 123),
96102
new SearchShardTarget("node1", new ShardId("idx", "na", shardId), null, OriginalIndices.NONE));
97103
SortField sortField = new SortField("timestamp", SortField.Type.LONG);
98-
queryResult.topDocs(new TopDocsAndMaxScore(new TopFieldDocs(
99-
new TotalHits(1, withScroll ? TotalHits.Relation.EQUAL_TO : TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO),
100-
new FieldDoc[] {
101-
new FieldDoc(randomInt(1000), Float.NaN, new Object[] { request.shardId().id() })
102-
}, new SortField[] { sortField }), Float.NaN),
103-
new DocValueFormat[] { DocValueFormat.RAW });
104+
if (withCollapse) {
105+
queryResult.topDocs(new TopDocsAndMaxScore(
106+
new CollapseTopFieldDocs(
107+
"collapse_field",
108+
new TotalHits(1, withScroll ? TotalHits.Relation.EQUAL_TO : TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO),
109+
new FieldDoc[]{
110+
new FieldDoc(randomInt(1000), Float.NaN, new Object[]{request.shardId().id()})
111+
},
112+
new SortField[]{sortField}, new Object[] { 0L }), Float.NaN),
113+
new DocValueFormat[]{DocValueFormat.RAW});
114+
} else {
115+
queryResult.topDocs(new TopDocsAndMaxScore(new TopFieldDocs(
116+
new TotalHits(1, withScroll ? TotalHits.Relation.EQUAL_TO : TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO),
117+
new FieldDoc[]{
118+
new FieldDoc(randomInt(1000), Float.NaN, new Object[]{request.shardId().id()})
119+
}, new SortField[]{sortField}), Float.NaN),
120+
new DocValueFormat[]{DocValueFormat.RAW});
121+
}
104122
queryResult.from(0);
105123
queryResult.size(1);
106124
successfulOps.incrementAndGet();
@@ -122,6 +140,9 @@ public void sendExecuteQuery(Transport.Connection connection, ShardSearchRequest
122140
} else {
123141
searchRequest.source().trackTotalHitsUpTo(2);
124142
}
143+
if (withCollapse) {
144+
searchRequest.source().collapse(new CollapseBuilder("collapse_field"));
145+
}
125146
searchRequest.allowPartialSearchResults(false);
126147
SearchPhaseController controller = new SearchPhaseController(
127148
writableRegistry(), r -> InternalAggregationTestCase.emptyReduceContextBuilder());
@@ -150,7 +171,11 @@ public void run() {
150171
assertThat(numWithTopDocs.get(), equalTo(0));
151172
} else {
152173
assertTrue(canReturnNullResponse.get());
153-
assertThat(numWithTopDocs.get(), greaterThanOrEqualTo(1));
174+
if (withCollapse) {
175+
assertThat(numWithTopDocs.get(), equalTo(0));
176+
} else {
177+
assertThat(numWithTopDocs.get(), greaterThanOrEqualTo(1));
178+
}
154179
}
155180
SearchPhaseController.ReducedQueryPhase phase = action.results.reduce();
156181
assertThat(phase.numReducePhases, greaterThanOrEqualTo(1));
@@ -163,6 +188,8 @@ public void run() {
163188
}
164189
assertThat(phase.sortedTopDocs.scoreDocs.length, equalTo(1));
165190
assertThat(phase.sortedTopDocs.scoreDocs[0], instanceOf(FieldDoc.class));
191+
assertThat(phase.sortedTopDocs.scoreDocs.length, equalTo(1));
192+
assertThat(phase.sortedTopDocs.scoreDocs[0], instanceOf(FieldDoc.class));
166193
assertThat(((FieldDoc) phase.sortedTopDocs.scoreDocs[0]).fields.length, equalTo(1));
167194
assertThat(((FieldDoc) phase.sortedTopDocs.scoreDocs[0]).fields[0], equalTo(0));
168195
}

0 commit comments

Comments
 (0)