Fallback to sub-aggregation if composite aggregation doesn't support by qianheng-aws · Pull Request #4413 · opensearch-project/sql

qianheng-aws · 2025-09-30T05:25:11Z

Description

Fallback to sub-aggregation if composite aggregation doesn't support(e.g. auto_span).

As sub-aggregation includes term agg which doesn't include null bucket, so we only fallback to push the aggregation when bucket_nullable=false. Otherwise, the aggregation will fail to push down and executes as a Calcite's Enumerable aggregate operator.

This PR also refact and scale the optimization #4337 for bucket aggregation including single bucket and nested bucket(i.e. sub-agg).

Related Issues

Resolves #4338

Check List

New functionality includes testing.
New functionality has been documented.
New functionality has javadoc added.
New functionality has a user manual doc added.
New PPL command checklist all confirmed.
API changes companion pull request created.
Commits are signed per the DCO using --signoff or -s.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Heng Qian <qianheng@amazon.com>

# Conflicts: # integ-test/src/test/resources/expectedOutput/calcite/explain_agg_sort_on_metrics2.json # integ-test/src/test/resources/expectedOutput/calcite/explain_limit_agg_pushdown_bucket_nullable1.json # integ-test/src/test/resources/expectedOutput/calcite/explain_limit_agg_pushdown_bucket_nullable2.json

Signed-off-by: Heng Qian <qianheng@amazon.com>

LantaoJin · 2025-10-09T08:25:57Z

...in/java/org/opensearch/sql/opensearch/planner/physical/OpenSearchAggregateIndexScanRule.java

+      final LogicalAggregate aggregate = call.rel(0);
+      final LogicalFilter filter = call.rel(1);
+      final LogicalProject project = call.rel(2);
+      final CalciteLogicalIndexScan scan = call.rel(3);


LogicalAggregate LogicalProject LogicalFilter(condition=[IS NOT NULL($7)])

Shouldn't be following?

final LogicalAggregate aggregate = call.rel(0); final LogicalProject project = call.rel(1); final LogicalFilter filter = call.rel(2); final CalciteLogicalIndexScan scan = call.rel(3);

The original plan is

LogicalAggregate LogicalProject LogicalFilter(condition=[IS NOT NULL($7)]) LogicalProject LogicalIndexScan

ProjectFilterTransposeRule and ProjectMergeRule will produce agg-filter-project-scan pattern.

Maybe FilterProjectTransposeRule + ProjectMergeRule may produce agg-project-filter-scan

The pattern agg-filter-project-scan will make it easier to detect whether the filter is derived from the agg. The other pattern could work as well while we need to do additional transformation since the ref of field has passed through a project operator.

LantaoJin · 2025-10-09T08:27:58Z

...in/java/org/opensearch/sql/opensearch/planner/physical/OpenSearchAggregateIndexScanRule.java

+    Config BUCKET_NON_NULL_AGG =
+        ImmutableOpenSearchAggregateIndexScanRule.Config.builder()
+            .build()
+            .withDescription("Agg-Filter-Project-TableScan")


why not "Agg-Project-Filter-TableScan"? I think Agg-Project should be treated as a single unit

But filter(isnotnull) here is also derived from the agg.

LantaoJin · 2025-10-09T08:30:00Z

opensearch/src/main/java/org/opensearch/sql/opensearch/request/AggregateAnalyzer.java

          return Pair.of(
-              ImmutableList.copyOf(newMetricBuilder.getAggregatorFactories()),
-              new CountAsTotalHitsParser(countAggNameList));
+              List.of(), new CountAsTotalHitsParser(countAggNameAndBuilderPair.getLeft()));


can you create a named var for countAggNameAndBuilderPair.getLeft()?

LantaoJin · 2025-10-09T08:30:53Z

opensearch/src/main/java/org/opensearch/sql/opensearch/request/AggregateAnalyzer.java

@@ -289,7 +275,7 @@ && isAutoDateSpan(project.getProjects().get(groupList.getFirst()))) {
   * @param countAllOnly remove count() only, or count(FIELD) will be removed.
   * @return a pair of removed ValueCountAggregationBuilder and updated metric builder


javadoc need to update

LantaoJin · 2025-10-09T08:32:14Z

opensearch/src/main/java/org/opensearch/sql/opensearch/request/AggregateAnalyzer.java

    }
  }

+  public static class CompositeUnSupportedException extends RuntimeException {


can you move it to package org.opensearch.sql.exception?

This exception only happens here and will be caught internally. It doesn't need to be accessed outside of this file

Signed-off-by: Heng Qian <qianheng@amazon.com>

opensearch-trigger-bot · 2025-10-10T05:13:02Z

The backport to 2.19-dev failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/sql/backport-2.19-dev 2.19-dev
# Navigate to the new working tree
pushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-4413-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 abd5bd3337af63892ff8ee762ad3c5902fa87b2a
# Push it to GitHub
git push --set-upstream origin backport/backport-4413-to-2.19-dev
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/sql/backport-2.19-dev

Then, create a pull request where the base branch is 2.19-dev and the compare/head branch is backport/backport-4413-to-2.19-dev.

…pensearch-project#4413) * Fallback to sub-aggregation if composite aggregation doesn't support Signed-off-by: Heng Qian <qianheng@amazon.com> * merging main Signed-off-by: Heng Qian <qianheng@amazon.com> * Address comments Signed-off-by: Heng Qian <qianheng@amazon.com> * Address comments Signed-off-by: Heng Qian <qianheng@amazon.com> --------- Signed-off-by: Heng Qian <qianheng@amazon.com> (cherry picked from commit abd5bd3) Signed-off-by: Heng Qian <qianheng@amazon.com>

…on doesn't support #4413 (#4498) * Fallback to sub-aggregation if composite aggregation doesn't support (#4413) * Fallback to sub-aggregation if composite aggregation doesn't support Signed-off-by: Heng Qian <qianheng@amazon.com> * merging main Signed-off-by: Heng Qian <qianheng@amazon.com> * Address comments Signed-off-by: Heng Qian <qianheng@amazon.com> * Address comments Signed-off-by: Heng Qian <qianheng@amazon.com> --------- Signed-off-by: Heng Qian <qianheng@amazon.com> (cherry picked from commit abd5bd3) Signed-off-by: Heng Qian <qianheng@amazon.com> * fix compiling Signed-off-by: Heng Qian <qianheng@amazon.com> --------- Signed-off-by: Heng Qian <qianheng@amazon.com>

…pensearch-project#4413) * Fallback to sub-aggregation if composite aggregation doesn't support Signed-off-by: Heng Qian <qianheng@amazon.com> * merging main Signed-off-by: Heng Qian <qianheng@amazon.com> * Address comments Signed-off-by: Heng Qian <qianheng@amazon.com> * Address comments Signed-off-by: Heng Qian <qianheng@amazon.com> --------- Signed-off-by: Heng Qian <qianheng@amazon.com> Signed-off-by: Asif Bashar <asif.bashar@gmail.com>

Fallback to sub-aggregation if composite aggregation doesn't support

e0975b6

Signed-off-by: Heng Qian <qianheng@amazon.com>

qianheng-aws added enhancement New feature or request PPL Piped processing language aggregation calcite calcite migration releated pushdown pushdown related issues backport 2.19-dev labels Sep 30, 2025

yuancu previously approved these changes Sep 30, 2025

View reviewed changes

yuancu mentioned this pull request Sep 30, 2025

Pushdown case function in aggregations as range queries #4400

Merged

14 tasks

qianheng-aws added 2 commits October 9, 2025 10:24

merging main

2be7021

Signed-off-by: Heng Qian <qianheng@amazon.com>

qianheng-aws dismissed yuancu’s stale review via 2be7021 October 9, 2025 05:02

yuancu previously approved these changes Oct 9, 2025

View reviewed changes

LantaoJin reviewed Oct 9, 2025

View reviewed changes

Address comments

8cceb0b

Signed-off-by: Heng Qian <qianheng@amazon.com>

qianheng-aws dismissed yuancu’s stale review via 8cceb0b October 10, 2025 02:23

Address comments

7711a05

Signed-off-by: Heng Qian <qianheng@amazon.com>

LantaoJin approved these changes Oct 10, 2025

View reviewed changes

yuancu approved these changes Oct 10, 2025

View reviewed changes

yuancu merged commit abd5bd3 into opensearch-project:main Oct 10, 2025
33 checks passed

opensearch-trigger-bot bot added the backport-failed label Oct 10, 2025

This was referenced Oct 10, 2025

[Backport 2.19-dev]Fallback to sub-aggregation if composite aggregation doesn't support #4413 #4498

Merged

Fix mapping after aggregation push down #4499

Closed

qianheng-aws mentioned this pull request Oct 10, 2025

Fix mapping after aggregation push down #4500

Merged

8 tasks

LantaoJin added the backport-manually Filed a PR to backport manually. label Oct 10, 2025

qianheng-aws deleted the subagg branch March 26, 2026 10:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fallback to sub-aggregation if composite aggregation doesn't support#4413

Fallback to sub-aggregation if composite aggregation doesn't support#4413
yuancu merged 5 commits intoopensearch-project:mainfrom
qianheng-aws:subagg

qianheng-aws commented Sep 30, 2025 •

edited

Loading

Uh oh!

LantaoJin Oct 9, 2025

Uh oh!

qianheng-aws Oct 9, 2025

Uh oh!

qianheng-aws Oct 9, 2025 •

edited

Loading

Uh oh!

LantaoJin Oct 9, 2025

Uh oh!

qianheng-aws Oct 9, 2025

Uh oh!

LantaoJin Oct 9, 2025

Uh oh!

qianheng-aws Oct 10, 2025

Uh oh!

LantaoJin Oct 9, 2025

Uh oh!

qianheng-aws Oct 10, 2025

Uh oh!

LantaoJin Oct 9, 2025

Uh oh!

qianheng-aws Oct 10, 2025

Uh oh!

Uh oh!

opensearch-trigger-bot bot commented Oct 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -289,7 +275,7 @@ && isAutoDateSpan(project.getProjects().get(groupList.getFirst()))) {
		* @param countAllOnly remove count() only, or count(FIELD) will be removed.
		* @return a pair of removed ValueCountAggregationBuilder and updated metric builder

Conversation

qianheng-aws commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qianheng-aws Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

opensearch-trigger-bot bot commented Oct 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

qianheng-aws commented Sep 30, 2025 •

edited

Loading

qianheng-aws Oct 9, 2025 •

edited

Loading