Pārlūkot izejas kodu

Allow terms agg to default to depth first (#54845)

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.
Nik Everett 5 gadi atpakaļ
vecāks
revīzija
4c6184ba50

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

@@ -102,10 +102,10 @@ public class TermsAggregatorFactory extends ValuesSourceAggregatorFactory {
                 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?

+ 20 - 3
server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java

@@ -139,7 +139,7 @@ public class TermsAggregatorTests extends AggregatorTestCase {
             CoreValuesSourceType.BOOLEAN);
     }
 
-    public void testGlobalOrdinalsExecutionHint() throws Exception {
+    public void testUsesGlobalOrdinalsByDefault() throws Exception {
         randomizeAggregatorImpl = false;
 
         Directory directory = newDirectory();
@@ -150,8 +150,7 @@ public class TermsAggregatorTests extends AggregatorTestCase {
         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 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