Support composite aggregation paginating

[LantaoJin]

Description

Support composite aggregation paginating

This PR fix the correctness of following clickbench queries:
Q28, Q29: aggregate with having clause
Q42: offset exceeds the bucket size
and other queries such aggregations within join

Q28:

SELECT CounterID, AVG(length(URL)) AS l, COUNT(*) AS c
FROM hits
WHERE URL <> '' 
GROUP BY CounterID 
HAVING COUNT(*) > 100000 
ORDER BY l DESC 
LIMIT 25;

In PPL query, where c > 100000 actually is a HAVING clause:

source=hits
source=hits
| where URL != ''
| stats bucket_nullable=false avg(length(URL)) as l, count() as c by CounterID
| where c > 100000
| sort - l
| head 25

Q42:

SELECT WindowClientWidth, WindowClientHeight, COUNT(*) AS PageViews
FROM hits WHERE CounterID = 62 AND EventDate >= '2013-07-01' AND EventDate <= '2013-07-31'
AND IsRefresh = 0 AND DontCountHits = 0 AND URLHash = 2868770270353813622
GROUP BY WindowClientWidth, WindowClientHeight ORDER BY PageViews DESC LIMIT 10 OFFSET 10000;

In PPL query, the from 10000 exceeds the default bucket size

source=hits
| where CounterID = 62 and EventDate >= '2013-07-01 00:00:00' and EventDate <= '2013-07-31 00:00:00' and IsRefresh = 0 and DontCountHits = 0 and URLHash = 2868770270353813622
| stats bucket_nullable=false count() as PageViews by WindowClientWidth, WindowClientHeight
| sort - PageViews
| head 10 from 10000

Query aggregations within join:
SQL

SELECT
*
FROM (
  SELECT COUNT(*) cnt, DEPTNO 
  FROM EMP
 GROUP BY DEPTNO
) t1
INNER JOIN (
  SELECT COUNT(*) cnt, DEPTNO
  FROM DEPT
  GROUP BY DEPTNO
) t2 
ON t1.DEPTNO = t2.DEPTNO

PPL

source=EMP
| stats count() as cnt by DEPTNO 
| join type=inner DEPTNO [
  source=DEPT 
  | stats count() as cnt by DEPTNO
]

Related Issues

Resolves #4836

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.

Summary by CodeRabbit

Release Notes

• New Features

  • Added support for HAVING clause optimization in aggregation queries to improve query performance.
  • Implemented paginating aggregations for enhanced memory efficiency with large datasets.

• Tests

  • Expanded test coverage for aggregation queries with HAVING conditions and complex filtering scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Important

Review skipped

More than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review.

81 files out of 189 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This PR introduces support for pushing down HAVING clauses in Calcite query optimization and implements composite aggregation pagination. It adds a new HavingPushdownRule, modifies request/response handling to support paginating aggregations with afterKey state management, and includes comprehensive test coverage with expected output files for HAVING aggregation scenarios.

Changes

Cohort / File(s)	Summary
HAVING Pushdown Rule Implementation opensearch/src/main/java/org/opensearch/sql/opensearch/planner/rules/HavingPushdownRule.java	New rule class to match LogicalFilter → LogicalProject → CalciteLogicalIndexScan pattern and push down HAVING clauses via pushDownHavingClauseFlag. Includes Config interface with DEFAULT configuration.
Rule Registration opensearch/src/main/java/org/opensearch/sql/opensearch/planner/rules/OpenSearchIndexRules.java	Registered new HAVING_PUSHDOWN rule in OPEN_SEARCH_INDEX_SCAN_RULES list.
CalciteLogicalIndexScan HAVING Support opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/CalciteLogicalIndexScan.java	Added public method pushDownHavingClauseFlag(LogicalFilter filter) to check conditions and push down HAVING clauses on composite aggregations.
PushDownContext & PushDownType Updates opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/context/PushDownContext.java, opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/context/PushDownType.java	Added HAVING enum constant to PushDownType; added isHavingFlagPushed state flag to PushDownContext.
AggPushDownAction HAVING Flag opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/context/AggPushDownAction.java	Added hasHavingClause flag with setHavingClauseFlag() setter; updated apply() to pass boolean hasHavingClause to pushDownAggregation().
AbstractCalciteIndexScan Enhancements opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/AbstractCalciteIndexScan.java	Added public methods getAggMeasureNameList() and isHavingFlagPushed(); expanded cost/row-count estimation to include HAVING alongside FILTER.
OpenSearchRequestBuilder Paginating Aggregation opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java	Added paginatingAgg flag; extended build() pathway to handle paginating-agg flows with composite afterKey support; updated pushDownAggregation() signature to accept hasHavingClause boolean; added toString() override with paginatingAgg state.
OpenSearchQueryRequest Paginating Aggregation opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java	Added static factory methods of(...); new constructor with paginatingAgg parameter; refactored search() to support paginating-agg flow with afterKey collection and continuation; updated cleanup paths to reset afterKey.
OpenSearchIndex & OpenSearchResponse opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java, opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java	Added buildRequest(OpenSearchRequestBuilder) public API method to OpenSearchIndex; added noCompositeAfterKey() method to OpenSearchResponse for checking composite aggregation continuation.
OpenSearchIndexEnumerator Logic opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexEnumerator.java	Narrowed aggregation stop condition to check noCompositeAfterKey(); adjusted hits-size condition to require non-zero hits below maxResultWindow.
OpenSearchIndexScanAggregationBuilder opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanAggregationBuilder.java	Updated pushDownAggregation() call to pass false as second argument for hasHavingClause.
Integration Test: HAVING Explain Plans integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java	Added testHaving() method executing three explain queries with HAVING conditions on stats aggregations.
Integration Test: HAVING Stats Command integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteStatsCommandIT.java	Added testStatsHaving() method testing PPL stats query with HAVING filter; added @Override to init() method.
Expected Output: ClickBench Q28 & Q29 integ-test/src/test/resources/expectedOutput/calcite/clickbench/q28.yaml, integ-test/src/test/resources/expectedOutput/calcite/clickbench/q29.yaml	Updated physical plans to include HAVING filters in CalciteEnumerableIndexScan PushDownContext.
Expected Output: HAVING Aggregation Plans integ-test/src/test/resources/expectedOutput/calcite/explain_agg_having{1,2,3}.yaml	New YAML files containing complete logical and physical Calcite explain plans for three HAVING aggregation scenarios with composite bucket pagination.
Expected Output: Explain Output integ-test/src/test/resources/expectedOutput/calcite/explain_output.yaml	Updated physical plan with revised projections [avg_age, state], added HAVING IS NOT NULL($0), adjusted Sort and pagination, set paginatingAgg=true.
Unit Tests: Request/Builder Updates opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequestTest.java, opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java, opensearch/src/test/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanOptimizationTest.java	Replaced direct constructor calls with OpenSearchQueryRequest.of(...); updated pushDownAggregation() calls to pass false as second argument.
PPL Aggregation Tests ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLAggregationTest.java	Added three new test cases (testHaving1, testHaving2, testHaving3) covering HAVING semantics, bucket_nullable behavior, and complex filter expressions on aggregations.

Sequence Diagram

sequenceDiagram
    participant Optimizer
    participant HavingPushdown as HavingPushdownRule
    participant Scan as CalciteLogicalIndexScan
    participant Builder as OpenSearchRequestBuilder
    participant Query as OpenSearchQueryRequest
    participant Response as OpenSearchResponse
    participant Enumerator as OpenSearchIndexEnumerator

    Optimizer->>HavingPushdown: onMatch(LogicalFilter → Project → IndexScan)
    HavingPushdown->>Scan: pushDownHavingClauseFlag(filter)
    alt Conditions Met
        Scan->>Scan: Add HAVING to PushDownContext
        Scan-->>HavingPushdown: Return new scan with HAVING flag
        HavingPushdown-->>Optimizer: Transform plan with HAVING
    else Conditions Not Met
        Scan-->>HavingPushdown: Return null
    end

    Optimizer->>Builder: build()
    alt Has Aggregate with HAVING
        Builder->>Builder: Set paginatingAgg = true
        Builder->>Query: Create with paginatingAgg=true
    else Regular Path
        Builder->>Query: Create with paginatingAgg=false
    end

    Query->>Query: Execute search()
    alt paginatingAgg Flow
        Query->>Query: searchPaginatingAgg()
        Query->>Response: Capture CompositeAggregation.afterKey
        Response-->>Query: Return response with afterKey
        alt No More Results
            Response->>Response: noCompositeAfterKey()
            Response-->>Enumerator: true
            Enumerator->>Enumerator: Stop fetching batches
        else More Results
            Query->>Query: Set afterKey in CompositeAggregationBuilder
            Query->>Query: Execute next search
        end
    else Regular Flow
        Query->>Query: Single-page or PIT search
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Areas requiring extra attention:

• HavingPushdownRule.java: New rule implementation with pattern matching logic and scan transformation; validate correctness of filter extraction and HAVING flag application
• OpenSearchRequestBuilder & OpenSearchQueryRequest: Significant refactoring introducing paginatingAgg flag and searchPaginatingAgg() flow; verify afterKey state management, cleanup paths, and interaction with existing PIT logic
• OpenSearchIndexEnumerator: Modified stop conditions for aggregations; ensure noCompositeAfterKey() check correctly handles pagination and doesn't premature exit batches
• CalciteLogicalIndexScan.pushDownHavingClauseFlag(): Conditional HAVING pushdown requires validation that aggregate field checks and filter condition extraction are correct
• PushDownContext/AggPushDownAction coordination: Verify the HAVING flag threading through multiple layers (Rule → Scan → Context → Builder → Request)

Possibly related PRs

• #4867 — Modifies aggregation push-down logic in AggPushDownAction and request builder, directly impacted by this PR's changes to the push-down API signature.

Suggested labels

pushdown

Suggested reviewers

• penghuo
• ps48
• kavithacm
• derek-ho
• joshuali925
• noCharger
• dai-chen
• anirudha
• Yury-Fridlyand
• MaxKsyunz
• vamsimanohar
• acarbonetto
• seankao-az
• yuancu
• Swiddis

Poem

🐰 A HAVING clause hops through the scan,
Paginating buckets as fast as we can—
Composite afterKeys lead the way,
With filters pushed down every day,
Optimization dreams come true, hooray! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.63% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The title 'Support composite aggregation paginating' is partially related to the changeset but lacks clarity and doesn't fully capture the main objective of supporting HAVING clause pushdown with composite aggregation pagination.	Consider revising the title to be more specific and clear, such as 'Support composite aggregation pagination with HAVING clause pushdown' or 'Add HAVING pushdown rule for paginated composite aggregations'.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR implements composite aggregation pagination and HAVING-clause pushdown to fix incorrect query results in Q29 and Q42, directly addressing the requirements in issue #4836.
Out of Scope Changes check	✅ Passed	All changes are directly related to implementing composite aggregation pagination and HAVING-clause support. No unrelated or extraneous modifications were detected.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[LantaoJin]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[LantaoJin]

remove all needClean and searchDone in plan.

[LantaoJin]

Add CalcitePPLAggregationPaginatingIT and CalcitePPLTpchPaginatingIT to verifiy the results have no changes with paginating feature.

[LantaoJin]

main change in yarm files is updating requestedTotalSize to limited value for composite aggregation.

[opensearch-trigger-bot]

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-4884-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 9930665c372f433eea4aeb04b5a4cfcd51be3e9e
# Push it to GitHub
git push --set-upstream origin backport/backport-4884-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-4884-to-2.19-dev.

[opensearch-trigger-bot]

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-4884-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 9930665c372f433eea4aeb04b5a4cfcd51be3e9e
# Push it to GitHub
git push --set-upstream origin backport/backport-4884-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-4884-to-2.19-dev.