From cd379553ded24566b77f10ba840f45a9b6ea3538 Mon Sep 17 00:00:00 2001 From: bowenlan-amzn Date: Fri, 5 Jan 2024 10:32:27 -0800 Subject: [PATCH] address comments Signed-off-by: bowenlan-amzn --- .../aggregations/bucket/FastFilterRewriteHelper.java | 2 +- .../bucket/composite/CompositeAggregator.java | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/FastFilterRewriteHelper.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/FastFilterRewriteHelper.java index 1d1e7a5fe7862..59781aff9b822 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/FastFilterRewriteHelper.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/FastFilterRewriteHelper.java @@ -297,7 +297,7 @@ public void setShowOtherBucket(boolean showOtherBucket) { this.showOtherBucket = showOtherBucket; } - public boolean isRewriteable(Object parent, int subAggLength) { + public boolean isRewriteable(final Object parent, final int subAggLength) { if (parent == null && subAggLength == 0 && !missing && !hasScript) { if (type == Type.FILTERS) { return !showOtherBucket; diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregator.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregator.java index 71249266c6bb3..e79e4602a3a2d 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregator.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregator.java @@ -165,8 +165,10 @@ final class CompositeAggregator extends BucketsAggregator { fastFilterContext = new FastFilterRewriteHelper.FastFilterContext(sourceConfigs, rawAfterKey, formats); if (fastFilterContext.isRewriteable(parent, subAggregators.length)) { + // Currently the filter rewrite is only supported for date histograms RoundingValuesSource dateHistogramSource = fastFilterContext.getDateHistogramSource(); preparedRounding = dateHistogramSource.getPreparedRounding(); + // bucketOrds is the data structure for saving date histogram results bucketOrds = LongKeyedBucketOrds.build(context.bigArrays(), CardinalityUpperBound.ONE); fastFilterContext.setSize(size); FastFilterRewriteHelper.buildFastFilter( @@ -237,15 +239,17 @@ public InternalAggregation[] buildAggregations(long[] owningBucketOrds) throws I // Build results from fast filters optimization if (bucketOrds != null) { + // CompositeKey is the value of bucket key final Map bucketMap = new HashMap<>(); + // Some segments may not be optimized, so buckets may contain results from the queue. for (InternalComposite.InternalBucket internalBucket : buckets) { bucketMap.put(internalBucket.getRawKey(), internalBucket); } - + // Loop over the buckets in the bucketOrds, and populate the map accordingly LongKeyedBucketOrds.BucketOrdsEnum ordsEnum = bucketOrds.ordsEnum(0); while (ordsEnum.next()) { - Long bucketValue = ordsEnum.value(); - CompositeKey key = new CompositeKey(bucketValue); + Long bucketKeyValue = ordsEnum.value(); + CompositeKey key = new CompositeKey(bucketKeyValue); if (bucketMap.containsKey(key)) { long docCount = bucketDocCount(ordsEnum.ord()) + bucketMap.get(key).getDocCount(); bucketMap.get(key).setDocCount(docCount); @@ -262,7 +266,7 @@ public InternalAggregation[] buildAggregations(long[] owningBucketOrds) throws I bucketMap.put(key, bucket); } } - + // since a map is not sorted structure, sort it before transform back to buckets List bucketList = new ArrayList<>(bucketMap.values()); CollectionUtil.introSort(bucketList, InternalComposite.InternalBucket::compareKey); buckets = bucketList.subList(0, Math.min(size, bucketList.size())).toArray(InternalComposite.InternalBucket[]::new);