diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorFactory.java index d6d393266881b..ed88022b3bdc4 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorFactory.java @@ -102,10 +102,10 @@ public Aggregator build(String name, if (valuesSource instanceof ValuesSource.Bytes.WithOrdinals == false) { execution = ExecutionMode.MAP; } - final long maxOrd = execution == ExecutionMode.GLOBAL_ORDINALS ? getMaxOrd(valuesSource, context.searcher()) : -1; if (execution == null) { execution = ExecutionMode.GLOBAL_ORDINALS; } + final long maxOrd = execution == ExecutionMode.GLOBAL_ORDINALS ? getMaxOrd(valuesSource, context.searcher()) : -1; if (subAggCollectMode == null) { subAggCollectMode = SubAggCollectionMode.DEPTH_FIRST; // TODO can we remove concept of AggregatorFactories.EMPTY? diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java index e48f6df687ef2..f345d1367a031 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java @@ -139,7 +139,7 @@ protected List getSupportedValuesSourceTypes() { CoreValuesSourceType.BOOLEAN); } - public void testGlobalOrdinalsExecutionHint() throws Exception { + public void testUsesGlobalOrdinalsByDefault() throws Exception { randomizeAggregatorImpl = false; Directory directory = newDirectory(); @@ -150,8 +150,7 @@ public void testGlobalOrdinalsExecutionHint() throws Exception { IndexSearcher indexSearcher = new IndexSearcher(indexReader); TermsAggregationBuilder aggregationBuilder = new TermsAggregationBuilder("_name").userValueTypeHint(ValueType.STRING) - .field("string") - .collectMode(Aggregator.SubAggCollectionMode.BREADTH_FIRST); + .field("string"); MappedFieldType fieldType = new KeywordFieldMapper.KeywordFieldType(); fieldType.setName("string"); fieldType.setHasDocValues(true); @@ -161,11 +160,29 @@ public void testGlobalOrdinalsExecutionHint() throws Exception { 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