Support nested aggregation when calcite enabled by LantaoJin · Pull Request #4979 · opensearch-project/sql

LantaoJin · 2025-12-19T09:59:29Z

Description

Refactor implementation for PPL: #3696 (closed)
Deprecated implementation for SQL: #2814 (closed)

Support nested aggregation in PPL only when calcite enabled.

With this PR, follow PPL query is able to execute a nested aggregation query.

source=logs | head 10000 | stats min(pages.load_time)

And it equals the DSL

GET logs/_search
{
  "aggs": {
    "pages": {
      "nested": {
        "path": "pages"
      },
      "aggs": {
        "min_load_time": { "min": { "field": "pages.load_time" } }
      }
    }
  }
}

Follow queries (group-by nested path) are supported with this PR as well:

source=test | top pages.load_time
source=test | stats count() by pages.load_time
source=test | dedup pages.load_time

Limitation:

PPL only
Calcite should be enabled
Throw UnsupportedOperationException if pushdown cannot be applied.
Follow queries (group-by nested root path) are supported with this PR without pushdown enahncement:

source=test | top pages
source=test | stats count() by pages
source=test | dedup pages

Related Issues

Resolves #4949, #4564, #2813 and #2529, and #2739

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: Lantao Jin <ltjin@amazon.com>

coderabbitai · 2025-12-19T09:59:37Z

📝 Walkthrough

Walkthrough

Adds nested-aggregation support and hint-based aggregation metadata across Calcite planning and OpenSearch pushdown layers, including nested-path resolution utilities, new hint utilities, a converter rule and physical nested aggregate, pushdown adaptations, response parsing updates, and integration tests/resources.

Changes

Cohort / File(s)	Summary
Calcite hint utils & visitor `core/src/main/java/org/opensearch/sql/calcite/utils/PPLHintUtils.java`, `core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java`	New PPLHintUtils to attach/query `ignoreNullBucket` and `hasNestedAggCall` hints; CalciteRelNodeVisitor now computes and propagates `hintIgnoreNullBucket`, detects nested aggregators, and applies both hints during aggregation planning.
Calcite prepare & removed strategy `core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java`, `core/src/main/java/org/opensearch/sql/calcite/utils/PPLHintStrategyTable.java`	Replaced protected factory API with public `createPrepare()`; removed standalone PPLHintStrategyTable (memoized table moved/handled in PPLHintUtils).
Plan utilities & schema helpers `core/src/main/java/org/opensearch/sql/calcite/utils/PlanUtils.java`, `core/src/main/java/org/opensearch/sql/utils/Utils.java`, `core/src/test/java/org/opensearch/sql/utils/UtilsTest.java`	Removed legacy add-ignore-null hint helper; added Utils.resolveNestedPath and zipWithIndex plus unit tests for nested-path resolution.
Index scan & rule registry `opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/CalciteLogicalIndexScan.java`, `opensearch/src/main/java/org/opensearch/sql/opensearch/planner/rules/OpenSearchIndexRules.java`	buildSchema() added to include nested-paths; pushdown rule registration updated to separate non-pushdown/pushdown rule sets; bucket nullability now derived via PPLHintUtils.
New nested-agg conversion & physical rel `opensearch/src/main/java/org/opensearch/sql/opensearch/planner/rules/EnumerableNestedAggregateRule.java`, `opensearch/src/main/java/org/opensearch/sql/opensearch/planner/physical/CalciteEnumerableNestedAggregate.java`	Added ConverterRule to convert hinted LogicalAggregate to a new CalciteEnumerableNestedAggregate physical rel (validation, cost tweak, implement() unimplemented).
Aggregate analysis & pushdown actions `opensearch/src/main/java/org/opensearch/sql/opensearch/request/AggregateAnalyzer.java`, `opensearch/src/main/java/org/opensearch/sql/opensearch/request/AggregateFilterAnalyzer.java`, `opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/context/AggPushDownAction.java`	Renamed aggFieldName→aggName; integrated Utils.resolveNestedPath to detect nested paths; AggPushDownAction unwraps/wraps NestedAggregationBuilder and adjusts composite/nested handling across push/re-push logic.
Predicate & response parsing `opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java`, `opensearch/src/main/java/org/opensearch/sql/opensearch/response/agg/BucketAggregationParser.java`, `opensearch/src/main/java/org/opensearch/sql/opensearch/response/agg/MetricParserHelper.java`	NamedFieldExpression gains nestedPath (via Utils.resolveNestedPath); SimpleQueryExpression emits nestedQuery when present; parsers unwrap Nested/InternalNested aggregations before parsing.
Dedup & other planner rule tweaks `opensearch/src/main/java/org/opensearch/sql/opensearch/planner/rules/DedupPushdownRule.java`	Dedup eligibility broadened to ARRAY/MAP; prevents pushdown when dedup columns resolve to nested paths; hint handling switched to PPLHintUtils.
Response/error handling & test support `opensearch/src/main/java/org/opensearch/sql/opensearch/response/error/ErrorMessageFactory.java`, `opensearch/src/test/java/org/opensearch/sql/opensearch/response/AggregationResponseUtils.java`	Improved cause unwrapping and wrapping for non-OpenSearch exceptions; AggregationResponseUtils registers Nested parsing support.
Docs, tests & resources docs/...; `integ-test/src/...` (many explain YAML/JSON, nested_simple mapping & data), `integ-test/src/test/java/.../CalcitePPLNestedAggregationIT.java`, `integ-test/src/test/java/.../CalciteExplainIT.java`, others	Added/updated integration tests and expected explain outputs for nested aggregation scenarios; adjusted various integration tests and fixtures; documentation updates to explain examples and limitations.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant CalciteRelNodeVisitor
    participant PPLHintUtils
    participant CalciteLogicalIndexScan
    participant EnumerableNestedAggregateRule
    participant AggregateAnalyzer
    participant OpenSearch

    Client->>CalciteRelNodeVisitor: visitAggregation(rel)
    CalciteRelNodeVisitor->>CalciteRelNodeVisitor: collect aggCallRefs
    CalciteRelNodeVisitor->>CalciteRelNodeVisitor: compute hintIgnoreNullBucket
    alt nested aggregator detected
        CalciteRelNodeVisitor->>PPLHintUtils: addNestedAggCallHintToAggregate(relBuilder)
    end
    alt hintIgnoreNullBucket true
        CalciteRelNodeVisitor->>PPLHintUtils: addIgnoreNullBucketHintToAggregate(relBuilder)
    end
    CalciteRelNodeVisitor->>CalciteLogicalIndexScan: pushDownAggregate(aggregate, hints)
    CalciteLogicalIndexScan->>CalciteLogicalIndexScan: buildSchema() / resolveNestedPath()
    CalciteLogicalIndexScan->>EnumerableNestedAggregateRule: rule matching (hints)
    EnumerableNestedAggregateRule->>EnumerableNestedAggregateRule: convert -> CalciteEnumerableNestedAggregate
    CalciteLogicalIndexScan->>AggregateAnalyzer: analyze(agg, bucketNames, nested info)
    AggregateAnalyzer->>AggregateAnalyzer: resolveNestedPath & build aggregations (may wrap NestedAggregationBuilder)
    CalciteLogicalIndexScan->>OpenSearch: execute pushdown with nested aggregation

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~90+ minutes

Possibly related PRs

Support pushdown dedup with expression #4957 — overlapping changes to DedupPushdownRule and hint/utility usage.
Prune old in operator push down rules #4992 — related modifications to Calcite hint and plan utility paths for ignore-null behavior.
Remove count aggregation for sort on aggregate measure #4867 — touches AggPushDownAction re-push and aggregation unwrapping logic.

Suggested labels

PPL, pushdown

Suggested reviewers

penghuo
anirudha
derek-ho
kavithacm
joshuali925

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 19.42% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Support nested aggregation when calcite enabled' accurately describes the main change: adding nested aggregation support for PPL when Calcite is enabled.
Linked Issues check	✅ Passed	The PR code changes implement nested aggregation support for PPL when Calcite is enabled, directly addressing issue #4949 which requires bucket-dependent aggregations (top, stats by, dedup) to work correctly on nested fields.
Out of Scope Changes check	✅ Passed	All changes are scoped to implementing nested aggregation support: hint utilities, rule additions, aggregation analyzers, schema handling, and comprehensive tests aligned with the PR objectives.
Description check	✅ Passed	The pull request description clearly explains the feature (nested aggregation support in PPL), provides examples of supported queries, documents known limitations, and references related issues.

✨ Finishing touches

📝 Generate docstrings

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

Signed-off-by: Lantao Jin <ltjin@amazon.com>

LantaoJin · 2025-12-20T03:25:11Z

core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java

-    protected Function0<CalcitePrepare> createPrepareFactory() {
-      return OpenSearchPrepareImpl::new;


After upgraded to 1.41. this method was not called any more, change it to createPrepare()

LantaoJin · 2025-12-20T03:27:00Z

core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java


    @Override
    protected PreparedResult implement(RelRoot root) {
-      Hook.PLAN_BEFORE_IMPLEMENTATION.run(root);


This hook was called twice for non-full-scannable plan

core/src/main/java/org/opensearch/sql/calcite/utils/PPLHintStrategyTable.java

LantaoJin · 2025-12-20T03:29:25Z

docs/user/ppl/interfaces/endpoint.md

 curl -sS -H 'Content-Type: application/json' \
 -X POST localhost:9200/_plugins/_ppl/_explain?format=extended \
-d '{"query" : "source=state_country | where age>30"}'
+-d '{"query" : "source=state_country | head 10 | where age>30"}'


avoid whole-plan-pushdown

LantaoJin · 2025-12-20T03:31:57Z

opensearch/src/main/java/org/opensearch/sql/opensearch/response/error/ErrorMessageFactory.java

    if (result.getCause() == null) {
+      if (result.getSuppressed().length > 0) {
+        return result.getSuppressed()[0];
+      }


For some exception without cause (root exception), the actual root cause may be attached in its suppressed cause.

LantaoJin · 2025-12-20T03:32:55Z

opensearch/src/main/java/org/opensearch/sql/opensearch/response/error/ErrorMessageFactory.java

      return new OpenSearchErrorMessage(exception, exception.status().getStatus());
    }
-    return new ErrorMessage(e, status);
+    return new ErrorMessage(cause, status);


Prefer to display the root cause instead of wrapping exception.

Signed-off-by: Lantao Jin <ltjin@amazon.com>

LantaoJin · 2025-12-30T06:24:52Z

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

        // Used to track the current sub-builder as analysis progresses
        Builder subBuilder = newMetricBuilder;
-        // Push auto date span & case in group-by list into nested aggregations
+        // Push auto date span & case in group-by list into structured aggregations


To avoid ambiguous, we call the multiple sub-aggregations structured aggregations instead of nested

LantaoJin · 2025-12-30T06:27:09Z

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


  private static Pair<Builder, List<MetricParser>> processAggregateCalls(
-      List<String> aggFieldNames,
+      List<String> aggNames,


Here the value could be min(a.b), count(c), rename aggFieldNames to aggNames.

opensearch-trigger-bot · 2026-01-05T02:47:36Z

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

) * refactor: throw exception if pushdown cannot be applied Signed-off-by: Lantao Jin <ltjin@amazon.com> * fix tests Signed-off-by: Lantao Jin <ltjin@amazon.com> * fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * Support top/dedup/aggregate by nested sub-fields Signed-off-by: Lantao Jin <ltjin@amazon.com> * fix typo Signed-off-by: Lantao Jin <ltjin@amazon.com> * address comments Signed-off-by: Lantao Jin <ltjin@amazon.com> * minor fixing Signed-off-by: Lantao Jin <ltjin@amazon.com> --------- Signed-off-by: Lantao Jin <ltjin@amazon.com> (cherry picked from commit 77633ef)

* refactor: throw exception if pushdown cannot be applied * fix tests * fix IT * Support top/dedup/aggregate by nested sub-fields * fix typo * address comments * minor fixing --------- (cherry picked from commit 77633ef) Signed-off-by: Lantao Jin <ltjin@amazon.com>

) * refactor: throw exception if pushdown cannot be applied Signed-off-by: Lantao Jin <ltjin@amazon.com> * fix tests Signed-off-by: Lantao Jin <ltjin@amazon.com> * fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * Support top/dedup/aggregate by nested sub-fields Signed-off-by: Lantao Jin <ltjin@amazon.com> * fix typo Signed-off-by: Lantao Jin <ltjin@amazon.com> * address comments Signed-off-by: Lantao Jin <ltjin@amazon.com> * minor fixing Signed-off-by: Lantao Jin <ltjin@amazon.com> --------- Signed-off-by: Lantao Jin <ltjin@amazon.com> Signed-off-by: Aaron Alvarez <aaarone@amazon.com>

refactor: throw exception if pushdown cannot be applied

e4d086a

Signed-off-by: Lantao Jin <ltjin@amazon.com>

LantaoJin mentioned this pull request Dec 19, 2025

Support nested aggregation when calcite enabled #3696

Closed

7 tasks

fix tests

8a3ead0

Signed-off-by: Lantao Jin <ltjin@amazon.com>

LantaoJin added the enhancement New feature or request label Dec 20, 2025

fix IT

7f98cc9

Signed-off-by: Lantao Jin <ltjin@amazon.com>

LantaoJin commented Dec 20, 2025

View reviewed changes

LantaoJin added 4 commits December 29, 2025 10:49

Merge remote-tracking branch 'upstream/main' into issues/2739_new

dfee375

Signed-off-by: Lantao Jin <ltjin@amazon.com>

Support top/dedup/aggregate by nested sub-fields

9fa0f38

Signed-off-by: Lantao Jin <ltjin@amazon.com>

Merge remote-tracking branch 'upstream/main' into issues/2739_new

7d05d10

Signed-off-by: Lantao Jin <ltjin@amazon.com>

fix typo

95ddffb

Signed-off-by: Lantao Jin <ltjin@amazon.com>

LantaoJin marked this pull request as ready for review December 29, 2025 14:03

LantaoJin requested review from GumpacG, MaxKsyunz, Swiddis, YANG-DB, Yury-Fridlyand, acarbonetto, anirudha, dai-chen, derek-ho, forestmvey, joshuali925, kavithacm, mengweieric, penghuo, ps48, seankao-az, vamsimanohar and ykmr1224 as code owners December 29, 2025 14:03

LantaoJin commented Dec 30, 2025

View reviewed changes

yuancu approved these changes Dec 30, 2025

View reviewed changes

qianheng-aws approved these changes Jan 5, 2026

View reviewed changes

qianheng-aws merged commit 77633ef into opensearch-project:main Jan 5, 2026
44 of 45 checks passed

qianheng-aws added the backport 2.19-dev label Jan 5, 2026

opensearch-trigger-bot bot added the backport-failed label Jan 5, 2026

LantaoJin mentioned this pull request Jan 5, 2026

[Backport 2.19-dev] Support nested aggregation when calcite enabled (#4979) #5012

Merged

LantaoJin added the backport-manually Filed a PR to backport manually. label Jan 5, 2026

This was referenced Jan 5, 2026

Introduce logical dedup operators for PPL #5014

Merged

Mvexpand feature #4944

Closed

Support UNION RECURSIVE in OpenSearch PPL #5027

Closed

Implement spath command with field resolution #5028

Merged

Feature/mvcombine #5025

Merged

This was referenced Jan 13, 2026

[Feature] implement transpose command as in the roadmap #4786 #5011

Merged

Push down filters on nested fields as nested queries #4825

Merged

Adding PPL convert command with numeric and memory conversion functions #5034

Closed

This was referenced Jan 20, 2026

Support spath with dynamic fields #5058

Merged

[BugFix] Fix the bug when boolean comparison condition is simplifed to field #5071

Merged

coderabbitai bot mentioned this pull request Jan 29, 2026

Add validation for expand command on scalar types (#5065) #5089

Closed

3 tasks

This was referenced Feb 16, 2026

[Feature] PPL Command: MvExpand #5144

Merged

[Feature] Calcite PPL search result highlighting #5141

Closed

feat: Implement PPL convert command with 5 conversion functions #5157

Merged

		protected Function0<CalcitePrepare> createPrepareFactory() {
		return OpenSearchPrepareImpl::new;

Conversation

LantaoJin commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

coderabbitai bot commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

LantaoJin Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

LantaoJin Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LantaoJin Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

LantaoJin Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

LantaoJin Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

LantaoJin Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

LantaoJin Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

opensearch-trigger-bot bot commented Jan 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LantaoJin commented Dec 19, 2025 •

edited

Loading

coderabbitai bot commented Dec 19, 2025 •

edited

Loading