From bc7c2fcf37cb61fc03593e4aec8c4c97915e0abf Mon Sep 17 00:00:00 2001 From: jimczi Date: Fri, 1 Feb 2019 11:16:17 +0100 Subject: [PATCH] Do not compute cardinality if the `terms` execution mode does not use `global_ordinals` 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 346da32763bd8..877a8e59bc2d3 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 @@ protected Aggregator doCreateInternal(ValuesSource valuesSource, Aggregator pare 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 @@ static SubAggCollectionMode subAggCollectionMode(int expectedSize, long maxOrd) } /** - * 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 @@ Aggregator create(String name, List pipelineAggregators, Map 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());