Skip to content

Commit 66e4fb4

Browse files
authored
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.
1 parent 2e475d6 commit 66e4fb4

File tree

1 file changed

+5
-16
lines changed

1 file changed

+5
-16
lines changed

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

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
package org.elasticsearch.search.aggregations.bucket.terms;
2121

2222
import org.apache.logging.log4j.LogManager;
23-
import org.apache.lucene.index.LeafReaderContext;
2423
import org.apache.lucene.search.IndexSearcher;
2524
import org.elasticsearch.common.ParseField;
2625
import org.elasticsearch.common.logging.DeprecationLogger;
@@ -134,7 +133,7 @@ protected Aggregator doCreateInternal(ValuesSource valuesSource, Aggregator pare
134133
if (valuesSource instanceof ValuesSource.Bytes.WithOrdinals == false) {
135134
execution = ExecutionMode.MAP;
136135
}
137-
final long maxOrd = getMaxOrd(context.searcher(), valuesSource, execution);
136+
final long maxOrd = execution == ExecutionMode.GLOBAL_ORDINALS ? getMaxOrd(valuesSource, context.searcher()) : -1;
138137
if (execution == null) {
139138
execution = ExecutionMode.GLOBAL_ORDINALS;
140139
}
@@ -208,23 +207,13 @@ static SubAggCollectionMode subAggCollectionMode(int expectedSize, long maxOrd)
208207
}
209208

210209
/**
211-
* Get the maximum ordinal value for the provided {@link ValuesSource} or -1
210+
* Get the maximum global ordinal value for the provided {@link ValuesSource} or -1
212211
* if the values source is not an instance of {@link ValuesSource.Bytes.WithOrdinals}.
213212
*/
214-
static long getMaxOrd(IndexSearcher searcher, ValuesSource source, ExecutionMode executionMode) throws IOException {
213+
static long getMaxOrd(ValuesSource source, IndexSearcher searcher) throws IOException {
215214
if (source instanceof ValuesSource.Bytes.WithOrdinals) {
216215
ValuesSource.Bytes.WithOrdinals valueSourceWithOrdinals = (ValuesSource.Bytes.WithOrdinals) source;
217-
if (executionMode == ExecutionMode.MAP) {
218-
// global ordinals are not requested so we don't load them
219-
// and return the biggest cardinality per segment instead.
220-
long maxOrd = -1;
221-
for (LeafReaderContext leaf : searcher.getIndexReader().leaves()) {
222-
maxOrd = Math.max(maxOrd, valueSourceWithOrdinals.ordinalsValues(leaf).getValueCount());
223-
}
224-
return maxOrd;
225-
} else {
226-
return valueSourceWithOrdinals.globalMaxOrd(searcher);
227-
}
216+
return valueSourceWithOrdinals.globalMaxOrd(searcher);
228217
} else {
229218
return -1;
230219
}
@@ -269,7 +258,7 @@ Aggregator create(String name,
269258
List<PipelineAggregator> pipelineAggregators,
270259
Map<String, Object> metaData) throws IOException {
271260

272-
final long maxOrd = getMaxOrd(context.searcher(), valuesSource, ExecutionMode.GLOBAL_ORDINALS);
261+
final long maxOrd = getMaxOrd(valuesSource, context.searcher());
273262
assert maxOrd != -1;
274263
final double ratio = maxOrd / ((double) context.searcher().getIndexReader().numDocs());
275264

0 commit comments

Comments
 (0)