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.
This commit is contained in:
Nik Everett 2020-04-07 14:11:34 -04:00 committed by GitHub
parent b8d2b952b8
commit 1798d6722b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 21 additions and 4 deletions

View File

@ -137,10 +137,10 @@ public class TermsAggregatorFactory extends ValuesSourceAggregatorFactory<Values
if (valuesSource instanceof ValuesSource.Bytes.WithOrdinals == false) {
execution = ExecutionMode.MAP;
}
final long maxOrd = execution == ExecutionMode.GLOBAL_ORDINALS ? getMaxOrd(valuesSource, searchContext.searcher()) : -1;
if (execution == null) {
execution = ExecutionMode.GLOBAL_ORDINALS;
}
final long maxOrd = execution == ExecutionMode.GLOBAL_ORDINALS ? getMaxOrd(valuesSource, searchContext.searcher()) : -1;
SubAggCollectionMode cm = collectMode;
if (cm == null) {
cm = SubAggCollectionMode.DEPTH_FIRST;

View File

@ -137,7 +137,7 @@ public class TermsAggregatorTests extends AggregatorTestCase {
CoreValuesSourceType.BYTES));
}
public void testGlobalOrdinalsExecutionHint() throws Exception {
public void testUsesGlobalOrdinalsByDefault() throws Exception {
randomizeAggregatorImpl = false;
Directory directory = newDirectory();
@ -148,8 +148,7 @@ public class TermsAggregatorTests extends AggregatorTestCase {
IndexSearcher indexSearcher = new IndexSearcher(indexReader);
TermsAggregationBuilder aggregationBuilder = new TermsAggregationBuilder("_name", ValueType.STRING)
.field("string")
.collectMode(Aggregator.SubAggCollectionMode.BREADTH_FIRST);
.field("string");
MappedFieldType fieldType = new KeywordFieldMapper.KeywordFieldType();
fieldType.setName("string");
fieldType.setHasDocValues(true);
@ -159,11 +158,29 @@ public class TermsAggregatorTests extends AggregatorTestCase {
GlobalOrdinalsStringTermsAggregator globalAgg = (GlobalOrdinalsStringTermsAggregator) aggregator;
assertFalse(globalAgg.remapGlobalOrds());
// Infers depth_first because the maxOrd is 0 which is less than the size
aggregationBuilder
.subAggregation(AggregationBuilders.cardinality("card").field("string"));
aggregator = createAggregator(aggregationBuilder, indexSearcher, fieldType);
assertThat(aggregator, instanceOf(GlobalOrdinalsStringTermsAggregator.class));
globalAgg = (GlobalOrdinalsStringTermsAggregator) aggregator;
assertThat(globalAgg.collectMode, equalTo(Aggregator.SubAggCollectionMode.DEPTH_FIRST));
assertTrue(globalAgg.remapGlobalOrds());
aggregationBuilder
.collectMode(Aggregator.SubAggCollectionMode.DEPTH_FIRST);
aggregator = createAggregator(aggregationBuilder, indexSearcher, fieldType);
assertThat(aggregator, instanceOf(GlobalOrdinalsStringTermsAggregator.class));
globalAgg = (GlobalOrdinalsStringTermsAggregator) aggregator;
assertThat(globalAgg.collectMode, equalTo(Aggregator.SubAggCollectionMode.DEPTH_FIRST));
assertTrue(globalAgg.remapGlobalOrds());
aggregationBuilder
.collectMode(Aggregator.SubAggCollectionMode.BREADTH_FIRST);
aggregator = createAggregator(aggregationBuilder, indexSearcher, fieldType);
assertThat(aggregator, instanceOf(GlobalOrdinalsStringTermsAggregator.class));
globalAgg = (GlobalOrdinalsStringTermsAggregator) aggregator;
assertThat(globalAgg.collectMode, equalTo(Aggregator.SubAggCollectionMode.BREADTH_FIRST));
assertFalse(globalAgg.remapGlobalOrds());
aggregationBuilder