Skip to content

Commit 1798d67

Browse files
authored
Allow terms agg to default to depth first (#54845) (#54885)
If you didn't explictly set `global_ordinals` execution mode we were never collecting the information that we needed to select `depth_first` based on the request so we were always defaulting to `breadth_first`. This fixes it so we collect the information.
1 parent b8d2b95 commit 1798d67

File tree

2 files changed

+21
-4
lines changed

2 files changed

+21
-4
lines changed

server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,10 @@ protected Aggregator doCreateInternal(ValuesSource valuesSource,
137137
if (valuesSource instanceof ValuesSource.Bytes.WithOrdinals == false) {
138138
execution = ExecutionMode.MAP;
139139
}
140-
final long maxOrd = execution == ExecutionMode.GLOBAL_ORDINALS ? getMaxOrd(valuesSource, searchContext.searcher()) : -1;
141140
if (execution == null) {
142141
execution = ExecutionMode.GLOBAL_ORDINALS;
143142
}
143+
final long maxOrd = execution == ExecutionMode.GLOBAL_ORDINALS ? getMaxOrd(valuesSource, searchContext.searcher()) : -1;
144144
SubAggCollectionMode cm = collectMode;
145145
if (cm == null) {
146146
cm = SubAggCollectionMode.DEPTH_FIRST;

server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ protected List<ValuesSourceType> getSupportedValuesSourceTypes() {
137137
CoreValuesSourceType.BYTES));
138138
}
139139

140-
public void testGlobalOrdinalsExecutionHint() throws Exception {
140+
public void testUsesGlobalOrdinalsByDefault() throws Exception {
141141
randomizeAggregatorImpl = false;
142142

143143
Directory directory = newDirectory();
@@ -148,8 +148,7 @@ public void testGlobalOrdinalsExecutionHint() throws Exception {
148148
IndexSearcher indexSearcher = new IndexSearcher(indexReader);
149149

150150
TermsAggregationBuilder aggregationBuilder = new TermsAggregationBuilder("_name", ValueType.STRING)
151-
.field("string")
152-
.collectMode(Aggregator.SubAggCollectionMode.BREADTH_FIRST);
151+
.field("string");
153152
MappedFieldType fieldType = new KeywordFieldMapper.KeywordFieldType();
154153
fieldType.setName("string");
155154
fieldType.setHasDocValues(true);
@@ -159,11 +158,29 @@ public void testGlobalOrdinalsExecutionHint() throws Exception {
159158
GlobalOrdinalsStringTermsAggregator globalAgg = (GlobalOrdinalsStringTermsAggregator) aggregator;
160159
assertFalse(globalAgg.remapGlobalOrds());
161160

161+
// Infers depth_first because the maxOrd is 0 which is less than the size
162162
aggregationBuilder
163163
.subAggregation(AggregationBuilders.cardinality("card").field("string"));
164164
aggregator = createAggregator(aggregationBuilder, indexSearcher, fieldType);
165165
assertThat(aggregator, instanceOf(GlobalOrdinalsStringTermsAggregator.class));
166166
globalAgg = (GlobalOrdinalsStringTermsAggregator) aggregator;
167+
assertThat(globalAgg.collectMode, equalTo(Aggregator.SubAggCollectionMode.DEPTH_FIRST));
168+
assertTrue(globalAgg.remapGlobalOrds());
169+
170+
aggregationBuilder
171+
.collectMode(Aggregator.SubAggCollectionMode.DEPTH_FIRST);
172+
aggregator = createAggregator(aggregationBuilder, indexSearcher, fieldType);
173+
assertThat(aggregator, instanceOf(GlobalOrdinalsStringTermsAggregator.class));
174+
globalAgg = (GlobalOrdinalsStringTermsAggregator) aggregator;
175+
assertThat(globalAgg.collectMode, equalTo(Aggregator.SubAggCollectionMode.DEPTH_FIRST));
176+
assertTrue(globalAgg.remapGlobalOrds());
177+
178+
aggregationBuilder
179+
.collectMode(Aggregator.SubAggCollectionMode.BREADTH_FIRST);
180+
aggregator = createAggregator(aggregationBuilder, indexSearcher, fieldType);
181+
assertThat(aggregator, instanceOf(GlobalOrdinalsStringTermsAggregator.class));
182+
globalAgg = (GlobalOrdinalsStringTermsAggregator) aggregator;
183+
assertThat(globalAgg.collectMode, equalTo(Aggregator.SubAggCollectionMode.BREADTH_FIRST));
167184
assertFalse(globalAgg.remapGlobalOrds());
168185

169186
aggregationBuilder

0 commit comments

Comments
 (0)