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
dd6043c1c0 (r252984782) added after the merge.
This commit is contained in:
Jim Ferenczi 2019-02-01 15:32:19 +01:00 committed by GitHub
parent 2e475d63f7
commit 66e4fb4fb6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 5 additions and 16 deletions

View File

@ -20,7 +20,6 @@
package org.elasticsearch.search.aggregations.bucket.terms; package org.elasticsearch.search.aggregations.bucket.terms;
import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.LogManager;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.IndexSearcher;
import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.DeprecationLogger;
@ -134,7 +133,7 @@ public class TermsAggregatorFactory extends ValuesSourceAggregatorFactory<Values
if (valuesSource instanceof ValuesSource.Bytes.WithOrdinals == false) { if (valuesSource instanceof ValuesSource.Bytes.WithOrdinals == false) {
execution = ExecutionMode.MAP; 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) { if (execution == null) {
execution = ExecutionMode.GLOBAL_ORDINALS; 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}. * 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) { if (source instanceof ValuesSource.Bytes.WithOrdinals) {
ValuesSource.Bytes.WithOrdinals valueSourceWithOrdinals = (ValuesSource.Bytes.WithOrdinals) source; 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 { } else {
return -1; return -1;
} }
@ -269,7 +258,7 @@ public class TermsAggregatorFactory extends ValuesSourceAggregatorFactory<Values
List<PipelineAggregator> pipelineAggregators, List<PipelineAggregator> pipelineAggregators,
Map<String, Object> metaData) throws IOException { 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; assert maxOrd != -1;
final double ratio = maxOrd / ((double) context.searcher().getIndexReader().numDocs()); final double ratio = maxOrd / ((double) context.searcher().getIndexReader().numDocs());