Remove useless aggregation helper (#58571) (#58578)

`descendsFromBucketAggregator` was important before we removed
`asMultiBucketAggregator` but now that it is gone
`collectsFromSingleBucket` is good enough.

Relates to #56487

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Nik Everett 2020-06-26 15:58:44 -04:00 committed by GitHub
parent 775fb5d4cf
commit 67e9d39932
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 11 additions and 27 deletions

View File

@ -28,7 +28,6 @@ import org.elasticsearch.common.lease.Releasable;
import org.elasticsearch.common.xcontent.DeprecationHandler;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.search.aggregations.bucket.BucketsAggregator;
import org.elasticsearch.search.aggregations.support.AggregationPath;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.search.sort.SortOrder;
@ -64,19 +63,6 @@ public abstract class Aggregator extends BucketCollector implements Releasable {
AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException;
}
/**
* Returns whether one of the parents is a {@link BucketsAggregator}.
*/
public static boolean descendsFromBucketAggregator(Aggregator parent) {
while (parent != null) {
if (parent instanceof BucketsAggregator) {
return true;
}
parent = parent.parent();
}
return false;
}
/**
* Return the name of this aggregator.
*/

View File

@ -296,15 +296,13 @@ public class SignificantTermsAggregatorFactory extends ValuesSourceAggregatorFac
final IncludeExclude.OrdinalsFilter filter = includeExclude == null ? null : includeExclude.convertToOrdinalsFilter(format);
boolean remapGlobalOrd = true;
if (Aggregator.descendsFromBucketAggregator(parent) == false &&
factories == AggregatorFactories.EMPTY &&
includeExclude == null) {
/**
if (collectsFromSingleBucket && factories == AggregatorFactories.EMPTY && includeExclude == null) {
/*
* We don't need to remap global ords iff this aggregator:
* - is not a child of a bucket aggregator AND
* - collects from a single bucket AND
* - has no include/exclude rules AND
* - has no sub-aggregator
**/
*/
remapGlobalOrd = false;
}

View File

@ -351,14 +351,14 @@ public class TermsAggregatorFactory extends ValuesSourceAggregatorFactory {
if (factories == AggregatorFactories.EMPTY &&
includeExclude == null &&
Aggregator.descendsFromBucketAggregator(parent) == false &&
collectsFromSingleBucket &&
ordinalsValuesSource.supportsGlobalOrdinalsMapping() &&
// we use the static COLLECT_SEGMENT_ORDS to allow tests to force specific optimizations
(COLLECT_SEGMENT_ORDS!= null ? COLLECT_SEGMENT_ORDS.booleanValue() : ratio <= 0.5 && maxOrd <= 2048)) {
/**
/*
* We can use the low cardinality execution mode iff this aggregator:
* - has no sub-aggregator AND
* - is not a child of a bucket aggregator AND
* - collects from a single bucket AND
* - has a values source that can map from segment to global ordinals
* - At least we reduce the number of global ordinals look-ups by half (ration <= 0.5) AND
* - the maximum global ordinal is less than 2048 (LOW_CARDINALITY has additional memory usage,
@ -382,16 +382,16 @@ public class TermsAggregatorFactory extends ValuesSourceAggregatorFactory {
} else {
remapGlobalOrds = true;
if (includeExclude == null &&
Aggregator.descendsFromBucketAggregator(parent) == false &&
collectsFromSingleBucket &&
(factories == AggregatorFactories.EMPTY ||
(isAggregationSort(order) == false && subAggCollectMode == SubAggCollectionMode.BREADTH_FIRST))) {
/**
/*
* We don't need to remap global ords iff this aggregator:
* - has no include/exclude rules AND
* - is not a child of a bucket aggregator AND
* - only collects from a single bucket AND
* - has no sub-aggregator or only sub-aggregator that can be deferred
* ({@link SubAggCollectionMode#BREADTH_FIRST}).
**/
*/
remapGlobalOrds = false;
}
}