From 66e4fb4fb6e1b4cd1224409cb2f0541bcfb1900f Mon Sep 17 00:00:00 2001
From: Jim Ferenczi <jim.ferenczi@elastic.co>
Date: Fri, 1 Feb 2019 15:32:19 +0100
Subject: [PATCH] Do not compute cardinality if the `terms` execution mode does
 not use `global_ordinals` (#38169)

In #38158 we ensured that global ordinals are not loaded when another execution hint is explicitly set on the source. This change is a follow up that addresses a comment
https://github.com/elastic/elasticsearch/pull/38158/files/dd6043c1c019974fe1c58810384b89e30cd8b89b#r252984782 added after the merge.
---
 .../bucket/terms/TermsAggregatorFactory.java  | 21 +++++--------------
 1 file changed, 5 insertions(+), 16 deletions(-)

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 346da32763b..877a8e59bc2 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
@@ -20,7 +20,6 @@
 package org.elasticsearch.search.aggregations.bucket.terms;
 
 import org.apache.logging.log4j.LogManager;
-import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.IndexSearcher;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.logging.DeprecationLogger;
@@ -134,7 +133,7 @@ public class TermsAggregatorFactory extends ValuesSourceAggregatorFactory<Values
             if (valuesSource instanceof ValuesSource.Bytes.WithOrdinals == false) {
                 execution = ExecutionMode.MAP;
             }
-            final long maxOrd = getMaxOrd(context.searcher(), valuesSource, execution);
+            final long maxOrd = execution == ExecutionMode.GLOBAL_ORDINALS ? getMaxOrd(valuesSource, context.searcher()) : -1;
             if (execution == null) {
                 execution = ExecutionMode.GLOBAL_ORDINALS;
             }
@@ -208,23 +207,13 @@ public class TermsAggregatorFactory extends ValuesSourceAggregatorFactory<Values
     }
 
     /**
-     * Get the maximum ordinal value for the provided {@link ValuesSource} or -1
+     * Get the maximum global ordinal value for the provided {@link ValuesSource} or -1
      * if the values source is not an instance of {@link ValuesSource.Bytes.WithOrdinals}.
      */
-    static long getMaxOrd(IndexSearcher searcher, ValuesSource source, ExecutionMode executionMode) throws IOException {
+    static long getMaxOrd(ValuesSource source, IndexSearcher searcher) throws IOException {
         if (source instanceof ValuesSource.Bytes.WithOrdinals) {
             ValuesSource.Bytes.WithOrdinals valueSourceWithOrdinals = (ValuesSource.Bytes.WithOrdinals) source;
-            if (executionMode == ExecutionMode.MAP) {
-                // global ordinals are not requested so we don't load them
-                // and return the biggest cardinality per segment instead.
-                long maxOrd = -1;
-                for (LeafReaderContext leaf : searcher.getIndexReader().leaves()) {
-                    maxOrd = Math.max(maxOrd, valueSourceWithOrdinals.ordinalsValues(leaf).getValueCount());
-                }
-                return maxOrd;
-            } else {
-                return valueSourceWithOrdinals.globalMaxOrd(searcher);
-            }
+            return valueSourceWithOrdinals.globalMaxOrd(searcher);
         } else {
             return -1;
         }
@@ -269,7 +258,7 @@ public class TermsAggregatorFactory extends ValuesSourceAggregatorFactory<Values
                               List<PipelineAggregator> pipelineAggregators,
                               Map<String, Object> metaData) throws IOException {
 
-                final long maxOrd = getMaxOrd(context.searcher(), valuesSource, ExecutionMode.GLOBAL_ORDINALS);
+                final long maxOrd = getMaxOrd(valuesSource, context.searcher());
                 assert maxOrd != -1;
                 final double ratio = maxOrd / ((double) context.searcher().getIndexReader().numDocs());