diff --git a/CHANGELOG.md b/CHANGELOG.md index 00e55d11643d2..78711edab6c9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -134,6 +134,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fixed bug for searchable snapshot to take 'base_path' of blob into account ([#6558](https://github.com/opensearch-project/OpenSearch/pull/6558)) - Fix fuzziness validation ([#5805](https://github.com/opensearch-project/OpenSearch/pull/5805)) - Fix GetSnapshots to not return non-existent snapshots with ignore_unavailable=true ([#6839](https://github.com/opensearch-project/OpenSearch/pull/6839)) +- Fix GlobalAggregation with profile option enabled returns incorrect result ([#7114](https://github.com/opensearch-project/OpenSearch/pull/7114)) ### Security diff --git a/server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/GlobalIT.java b/server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/GlobalIT.java index 46e5fb7332e89..8a97d9c9e75dd 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/GlobalIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/GlobalIT.java @@ -83,10 +83,19 @@ public void setupSuiteScopeCluster() throws Exception { ensureSearchable(); } - public void testWithStatsSubAggregator() throws Exception { + public void testWithStatsSubAggregatorAndProfileEnabled() throws Exception { + testWithStatsSubAggregator(true); + } + + public void testWithStatsSubAggregatorAndProfileDisabled() throws Exception { + testWithStatsSubAggregator(false); + } + + private void testWithStatsSubAggregator(boolean profileEnabled) throws Exception { SearchResponse response = client().prepareSearch("idx") .setQuery(QueryBuilders.termQuery("tag", "tag1")) .addAggregation(global("global").subAggregation(stats("value_stats").field("value"))) + .setProfile(profileEnabled) .get(); assertSearchResponse(response); diff --git a/server/src/main/java/org/opensearch/search/aggregations/AggregationPhase.java b/server/src/main/java/org/opensearch/search/aggregations/AggregationPhase.java index 946da6e9c3369..fe7f90703f776 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/AggregationPhase.java +++ b/server/src/main/java/org/opensearch/search/aggregations/AggregationPhase.java @@ -38,6 +38,7 @@ import org.opensearch.common.lucene.search.Queries; import org.opensearch.search.aggregations.bucket.global.GlobalAggregator; import org.opensearch.search.internal.SearchContext; +import org.opensearch.search.profile.aggregation.ProfilingAggregator; import org.opensearch.search.profile.query.CollectorResult; import org.opensearch.search.profile.query.InternalProfileCollector; import org.opensearch.search.query.QueryPhaseExecutionException; @@ -67,7 +68,7 @@ public void preProcess(SearchContext context) { AggregatorFactories factories = context.aggregations().factories(); aggregators = factories.createTopLevelAggregators(context); for (int i = 0; i < aggregators.length; i++) { - if (aggregators[i] instanceof GlobalAggregator == false) { + if (!isGlobalAggregator(context, aggregators[i])) { collectors.add(aggregators[i]); } } @@ -106,7 +107,7 @@ public void execute(SearchContext context) { Aggregator[] aggregators = context.aggregations().aggregators(); List globals = new ArrayList<>(); for (int i = 0; i < aggregators.length; i++) { - if (aggregators[i] instanceof GlobalAggregator) { + if (isGlobalAggregator(context, aggregators[i])) { globals.add(aggregators[i]); } } @@ -168,4 +169,16 @@ private Collector createCollector(SearchContext context, List collec } return collector; } + + /** + * Checks if passed in aggregator is of type {@link GlobalAggregator}. This method takes care of Aggregator wrapped in + * {@link ProfilingAggregator} too + * @param context {@link SearchContext} + * @param aggregator input {@link Aggregator} instance to evaluate + * @return true input is {@link GlobalAggregator} instance or false otherwise + */ + private boolean isGlobalAggregator(SearchContext context, Aggregator aggregator) { + return (aggregator instanceof GlobalAggregator + || (context.getProfilers() != null && ProfilingAggregator.unwrap(aggregator) instanceof GlobalAggregator)); + } }