Pushdown sort aggregate metrics - #4603

Merged
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282
Oct 24, 2025
Merged

Pushdown sort aggregate metrics#4603
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282

Conversation

@LantaoJin

@LantaoJinLantaoJin commented Oct 20, 2025

Copy link
Copy Markdown
Member

Description

Pushdown sort aggregate metrics:

  • Add SORT_AGG_METRICS after AGGREGATION in pushdown context
  • Convert composite bucket agg to terms/histo/date-histo agg if sort one metric on single bucket
  • Convert composite bucket agg (all terms source) to multi-terms agg if sort one metric on multiple buckets

Related Issues

Resolves#4282

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>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin added the enhancement New feature or request label Oct 21, 2025
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin changed the title Pushdown sort aggregate metricsPushdown sort aggregate metrics for single bucketOct 24, 2025
return newTraitSet;
}

public CalciteLogicalIndexScan pushDownSortAggregateMetrics(Sort sort) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Will it be better to put these checks in one place if possible? Now the prerequisites are split both here and in aggAction.pushDownSortAggMetrics, although they are both aggregationBuilders and some checking seems overlapped.

@LantaoJinLantaoJin changed the title Pushdown sort aggregate metrics for single bucketPushdown sort aggregate metricsOct 24, 2025
// count agg optimized, get the path name from field names
path = fieldNames.get(collations.get(0).getFieldIndex());
} else if (metric instanceof ValuesSourceAggregationBuilder.LeafOnly) {
path = metric.getName();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit, non-blocking] Could this be combined with above branch? i.e. Does path = fieldNames.get(collations.get(0).getFieldIndex()); also works for metric instanceof ValuesSourceAggregationBuilder.LeafOnly?

List<String> fieldNames,
CompositeAggregationBuilder composite) {
String path;
AggregationBuilder metric = composite.getSubAggregations().stream().findFirst().orElse(null);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we only support only 1 agg metric for this enhancement? And anywhere to check this?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catching. added restricted checking.

Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJin
LantaoJin merged commit 5ebed84 into opensearch-project:mainOct 24, 2025
35 checks passed
@opensearch-trigger-bot

Copy link
Copy Markdown
Contributor

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 repositorycd$(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 treepushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-4603-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 5ebed8465dc9d498602a25b802207d63ee5030c8
# Push it to GitHub
git push --set-upstream origin backport/backport-4603-to-2.19-dev
# Go back to the original working treepopd# 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-4603-to-2.19-dev.

LantaoJin added a commit to LantaoJin/search-plugins-sql that referenced this pull request Oct 24, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
@LantaoJinLantaoJin added the backport-manually Filed a PR to backport manually. label Oct 24, 2025
yuancu pushed a commit that referenced this pull request Oct 27, 2025
* Pushdown sort aggregate metrics (#4603)
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
# Conflicts:
#	docs/user/ppl/functions/conversion.rst
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
Signed-off-by: Asif Bashar <asif.bashar@gmail.com>
expani pushed a commit to vinaykpud/sql that referenced this pull request Nov 4, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19-devbackport-failedbackport-manuallyFiled a PR to backport manually.enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Push down bucket aggregation with sorting by metrics to multi-terms aggregation

3 participants

@LantaoJin@yuancu@qianheng-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Pushdown sort aggregate metrics - #4603

Merged
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282
Oct 24, 2025
Merged

Pushdown sort aggregate metrics#4603
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282

Conversation

@LantaoJin

@LantaoJinLantaoJin commented Oct 20, 2025

Copy link
Copy Markdown
Member

Description

Pushdown sort aggregate metrics:

  • Add SORT_AGG_METRICS after AGGREGATION in pushdown context
  • Convert composite bucket agg to terms/histo/date-histo agg if sort one metric on single bucket
  • Convert composite bucket agg (all terms source) to multi-terms agg if sort one metric on multiple buckets

Related Issues

Resolves#4282

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>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin added the enhancement New feature or request label Oct 21, 2025
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin changed the title Pushdown sort aggregate metricsPushdown sort aggregate metrics for single bucketOct 24, 2025
return newTraitSet;
}

public CalciteLogicalIndexScan pushDownSortAggregateMetrics(Sort sort) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Will it be better to put these checks in one place if possible? Now the prerequisites are split both here and in aggAction.pushDownSortAggMetrics, although they are both aggregationBuilders and some checking seems overlapped.

@LantaoJinLantaoJin changed the title Pushdown sort aggregate metrics for single bucketPushdown sort aggregate metricsOct 24, 2025
// count agg optimized, get the path name from field names
path = fieldNames.get(collations.get(0).getFieldIndex());
} else if (metric instanceof ValuesSourceAggregationBuilder.LeafOnly) {
path = metric.getName();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit, non-blocking] Could this be combined with above branch? i.e. Does path = fieldNames.get(collations.get(0).getFieldIndex()); also works for metric instanceof ValuesSourceAggregationBuilder.LeafOnly?

List<String> fieldNames,
CompositeAggregationBuilder composite) {
String path;
AggregationBuilder metric = composite.getSubAggregations().stream().findFirst().orElse(null);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we only support only 1 agg metric for this enhancement? And anywhere to check this?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catching. added restricted checking.

Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJin
LantaoJin merged commit 5ebed84 into opensearch-project:mainOct 24, 2025
35 checks passed
@opensearch-trigger-bot

Copy link
Copy Markdown
Contributor

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 repositorycd$(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 treepushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-4603-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 5ebed8465dc9d498602a25b802207d63ee5030c8
# Push it to GitHub
git push --set-upstream origin backport/backport-4603-to-2.19-dev
# Go back to the original working treepopd# 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-4603-to-2.19-dev.

LantaoJin added a commit to LantaoJin/search-plugins-sql that referenced this pull request Oct 24, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
@LantaoJinLantaoJin added the backport-manually Filed a PR to backport manually. label Oct 24, 2025
yuancu pushed a commit that referenced this pull request Oct 27, 2025
* Pushdown sort aggregate metrics (#4603)
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
# Conflicts:
#	docs/user/ppl/functions/conversion.rst
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
Signed-off-by: Asif Bashar <asif.bashar@gmail.com>
expani pushed a commit to vinaykpud/sql that referenced this pull request Nov 4, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19-devbackport-failedbackport-manuallyFiled a PR to backport manually.enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Push down bucket aggregation with sorting by metrics to multi-terms aggregation

3 participants

@LantaoJin@yuancu@qianheng-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Pushdown sort aggregate metrics - #4603

Merged
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282
Oct 24, 2025
Merged

Pushdown sort aggregate metrics#4603
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282

Conversation

@LantaoJin

@LantaoJinLantaoJin commented Oct 20, 2025

Copy link
Copy Markdown
Member

Description

Pushdown sort aggregate metrics:

  • Add SORT_AGG_METRICS after AGGREGATION in pushdown context
  • Convert composite bucket agg to terms/histo/date-histo agg if sort one metric on single bucket
  • Convert composite bucket agg (all terms source) to multi-terms agg if sort one metric on multiple buckets

Related Issues

Resolves#4282

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>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin added the enhancement New feature or request label Oct 21, 2025
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin changed the title Pushdown sort aggregate metricsPushdown sort aggregate metrics for single bucketOct 24, 2025
return newTraitSet;
}

public CalciteLogicalIndexScan pushDownSortAggregateMetrics(Sort sort) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Will it be better to put these checks in one place if possible? Now the prerequisites are split both here and in aggAction.pushDownSortAggMetrics, although they are both aggregationBuilders and some checking seems overlapped.

@LantaoJinLantaoJin changed the title Pushdown sort aggregate metrics for single bucketPushdown sort aggregate metricsOct 24, 2025
// count agg optimized, get the path name from field names
path = fieldNames.get(collations.get(0).getFieldIndex());
} else if (metric instanceof ValuesSourceAggregationBuilder.LeafOnly) {
path = metric.getName();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit, non-blocking] Could this be combined with above branch? i.e. Does path = fieldNames.get(collations.get(0).getFieldIndex()); also works for metric instanceof ValuesSourceAggregationBuilder.LeafOnly?

List<String> fieldNames,
CompositeAggregationBuilder composite) {
String path;
AggregationBuilder metric = composite.getSubAggregations().stream().findFirst().orElse(null);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we only support only 1 agg metric for this enhancement? And anywhere to check this?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catching. added restricted checking.

Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJin
LantaoJin merged commit 5ebed84 into opensearch-project:mainOct 24, 2025
35 checks passed
@opensearch-trigger-bot

Copy link
Copy Markdown
Contributor

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 repositorycd$(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 treepushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-4603-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 5ebed8465dc9d498602a25b802207d63ee5030c8
# Push it to GitHub
git push --set-upstream origin backport/backport-4603-to-2.19-dev
# Go back to the original working treepopd# 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-4603-to-2.19-dev.

LantaoJin added a commit to LantaoJin/search-plugins-sql that referenced this pull request Oct 24, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
@LantaoJinLantaoJin added the backport-manually Filed a PR to backport manually. label Oct 24, 2025
yuancu pushed a commit that referenced this pull request Oct 27, 2025
* Pushdown sort aggregate metrics (#4603)
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
# Conflicts:
#	docs/user/ppl/functions/conversion.rst
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
Signed-off-by: Asif Bashar <asif.bashar@gmail.com>
expani pushed a commit to vinaykpud/sql that referenced this pull request Nov 4, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19-devbackport-failedbackport-manuallyFiled a PR to backport manually.enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Push down bucket aggregation with sorting by metrics to multi-terms aggregation

3 participants

@LantaoJin@yuancu@qianheng-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Pushdown sort aggregate metrics - #4603

Merged
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282
Oct 24, 2025
Merged

Pushdown sort aggregate metrics#4603
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282

Conversation

@LantaoJin

@LantaoJinLantaoJin commented Oct 20, 2025

Copy link
Copy Markdown
Member

Description

Pushdown sort aggregate metrics:

  • Add SORT_AGG_METRICS after AGGREGATION in pushdown context
  • Convert composite bucket agg to terms/histo/date-histo agg if sort one metric on single bucket
  • Convert composite bucket agg (all terms source) to multi-terms agg if sort one metric on multiple buckets

Related Issues

Resolves#4282

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>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin added the enhancement New feature or request label Oct 21, 2025
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin changed the title Pushdown sort aggregate metricsPushdown sort aggregate metrics for single bucketOct 24, 2025
return newTraitSet;
}

public CalciteLogicalIndexScan pushDownSortAggregateMetrics(Sort sort) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Will it be better to put these checks in one place if possible? Now the prerequisites are split both here and in aggAction.pushDownSortAggMetrics, although they are both aggregationBuilders and some checking seems overlapped.

@LantaoJinLantaoJin changed the title Pushdown sort aggregate metrics for single bucketPushdown sort aggregate metricsOct 24, 2025
// count agg optimized, get the path name from field names
path = fieldNames.get(collations.get(0).getFieldIndex());
} else if (metric instanceof ValuesSourceAggregationBuilder.LeafOnly) {
path = metric.getName();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit, non-blocking] Could this be combined with above branch? i.e. Does path = fieldNames.get(collations.get(0).getFieldIndex()); also works for metric instanceof ValuesSourceAggregationBuilder.LeafOnly?

List<String> fieldNames,
CompositeAggregationBuilder composite) {
String path;
AggregationBuilder metric = composite.getSubAggregations().stream().findFirst().orElse(null);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we only support only 1 agg metric for this enhancement? And anywhere to check this?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catching. added restricted checking.

Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJin
LantaoJin merged commit 5ebed84 into opensearch-project:mainOct 24, 2025
35 checks passed
@opensearch-trigger-bot

Copy link
Copy Markdown
Contributor

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 repositorycd$(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 treepushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-4603-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 5ebed8465dc9d498602a25b802207d63ee5030c8
# Push it to GitHub
git push --set-upstream origin backport/backport-4603-to-2.19-dev
# Go back to the original working treepopd# 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-4603-to-2.19-dev.

LantaoJin added a commit to LantaoJin/search-plugins-sql that referenced this pull request Oct 24, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
@LantaoJinLantaoJin added the backport-manually Filed a PR to backport manually. label Oct 24, 2025
yuancu pushed a commit that referenced this pull request Oct 27, 2025
* Pushdown sort aggregate metrics (#4603)
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
# Conflicts:
#	docs/user/ppl/functions/conversion.rst
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
Signed-off-by: Asif Bashar <asif.bashar@gmail.com>
expani pushed a commit to vinaykpud/sql that referenced this pull request Nov 4, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19-devbackport-failedbackport-manuallyFiled a PR to backport manually.enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Push down bucket aggregation with sorting by metrics to multi-terms aggregation

3 participants

@LantaoJin@yuancu@qianheng-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Pushdown sort aggregate metrics - #4603

Merged
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282
Oct 24, 2025
Merged

Pushdown sort aggregate metrics#4603
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282

Conversation

@LantaoJin

@LantaoJinLantaoJin commented Oct 20, 2025

Copy link
Copy Markdown
Member

Description

Pushdown sort aggregate metrics:

  • Add SORT_AGG_METRICS after AGGREGATION in pushdown context
  • Convert composite bucket agg to terms/histo/date-histo agg if sort one metric on single bucket
  • Convert composite bucket agg (all terms source) to multi-terms agg if sort one metric on multiple buckets

Related Issues

Resolves#4282

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>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin added the enhancement New feature or request label Oct 21, 2025
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin changed the title Pushdown sort aggregate metricsPushdown sort aggregate metrics for single bucketOct 24, 2025
return newTraitSet;
}

public CalciteLogicalIndexScan pushDownSortAggregateMetrics(Sort sort) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Will it be better to put these checks in one place if possible? Now the prerequisites are split both here and in aggAction.pushDownSortAggMetrics, although they are both aggregationBuilders and some checking seems overlapped.

@LantaoJinLantaoJin changed the title Pushdown sort aggregate metrics for single bucketPushdown sort aggregate metricsOct 24, 2025
// count agg optimized, get the path name from field names
path = fieldNames.get(collations.get(0).getFieldIndex());
} else if (metric instanceof ValuesSourceAggregationBuilder.LeafOnly) {
path = metric.getName();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit, non-blocking] Could this be combined with above branch? i.e. Does path = fieldNames.get(collations.get(0).getFieldIndex()); also works for metric instanceof ValuesSourceAggregationBuilder.LeafOnly?

List<String> fieldNames,
CompositeAggregationBuilder composite) {
String path;
AggregationBuilder metric = composite.getSubAggregations().stream().findFirst().orElse(null);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we only support only 1 agg metric for this enhancement? And anywhere to check this?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catching. added restricted checking.

Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJin
LantaoJin merged commit 5ebed84 into opensearch-project:mainOct 24, 2025
35 checks passed
@opensearch-trigger-bot

Copy link
Copy Markdown
Contributor

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 repositorycd$(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 treepushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-4603-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 5ebed8465dc9d498602a25b802207d63ee5030c8
# Push it to GitHub
git push --set-upstream origin backport/backport-4603-to-2.19-dev
# Go back to the original working treepopd# 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-4603-to-2.19-dev.

LantaoJin added a commit to LantaoJin/search-plugins-sql that referenced this pull request Oct 24, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
@LantaoJinLantaoJin added the backport-manually Filed a PR to backport manually. label Oct 24, 2025
yuancu pushed a commit that referenced this pull request Oct 27, 2025
* Pushdown sort aggregate metrics (#4603)
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
# Conflicts:
#	docs/user/ppl/functions/conversion.rst
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
Signed-off-by: Asif Bashar <asif.bashar@gmail.com>
expani pushed a commit to vinaykpud/sql that referenced this pull request Nov 4, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19-devbackport-failedbackport-manuallyFiled a PR to backport manually.enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Push down bucket aggregation with sorting by metrics to multi-terms aggregation

3 participants

@LantaoJin@yuancu@qianheng-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Pushdown sort aggregate metrics - #4603

Merged
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282
Oct 24, 2025
Merged

Pushdown sort aggregate metrics#4603
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282

Conversation

@LantaoJin

@LantaoJinLantaoJin commented Oct 20, 2025

Copy link
Copy Markdown
Member

Description

Pushdown sort aggregate metrics:

  • Add SORT_AGG_METRICS after AGGREGATION in pushdown context
  • Convert composite bucket agg to terms/histo/date-histo agg if sort one metric on single bucket
  • Convert composite bucket agg (all terms source) to multi-terms agg if sort one metric on multiple buckets

Related Issues

Resolves#4282

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>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin added the enhancement New feature or request label Oct 21, 2025
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin changed the title Pushdown sort aggregate metricsPushdown sort aggregate metrics for single bucketOct 24, 2025
return newTraitSet;
}

public CalciteLogicalIndexScan pushDownSortAggregateMetrics(Sort sort) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Will it be better to put these checks in one place if possible? Now the prerequisites are split both here and in aggAction.pushDownSortAggMetrics, although they are both aggregationBuilders and some checking seems overlapped.

@LantaoJinLantaoJin changed the title Pushdown sort aggregate metrics for single bucketPushdown sort aggregate metricsOct 24, 2025
// count agg optimized, get the path name from field names
path = fieldNames.get(collations.get(0).getFieldIndex());
} else if (metric instanceof ValuesSourceAggregationBuilder.LeafOnly) {
path = metric.getName();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit, non-blocking] Could this be combined with above branch? i.e. Does path = fieldNames.get(collations.get(0).getFieldIndex()); also works for metric instanceof ValuesSourceAggregationBuilder.LeafOnly?

List<String> fieldNames,
CompositeAggregationBuilder composite) {
String path;
AggregationBuilder metric = composite.getSubAggregations().stream().findFirst().orElse(null);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we only support only 1 agg metric for this enhancement? And anywhere to check this?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catching. added restricted checking.

Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJin
LantaoJin merged commit 5ebed84 into opensearch-project:mainOct 24, 2025
35 checks passed
@opensearch-trigger-bot

Copy link
Copy Markdown
Contributor

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 repositorycd$(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 treepushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-4603-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 5ebed8465dc9d498602a25b802207d63ee5030c8
# Push it to GitHub
git push --set-upstream origin backport/backport-4603-to-2.19-dev
# Go back to the original working treepopd# 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-4603-to-2.19-dev.

LantaoJin added a commit to LantaoJin/search-plugins-sql that referenced this pull request Oct 24, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
@LantaoJinLantaoJin added the backport-manually Filed a PR to backport manually. label Oct 24, 2025
yuancu pushed a commit that referenced this pull request Oct 27, 2025
* Pushdown sort aggregate metrics (#4603)
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
# Conflicts:
#	docs/user/ppl/functions/conversion.rst
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
Signed-off-by: Asif Bashar <asif.bashar@gmail.com>
expani pushed a commit to vinaykpud/sql that referenced this pull request Nov 4, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19-devbackport-failedbackport-manuallyFiled a PR to backport manually.enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Push down bucket aggregation with sorting by metrics to multi-terms aggregation

3 participants

@LantaoJin@yuancu@qianheng-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Pushdown sort aggregate metrics - #4603

Merged
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282
Oct 24, 2025
Merged

Pushdown sort aggregate metrics#4603
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282

Conversation

@LantaoJin

@LantaoJinLantaoJin commented Oct 20, 2025

Copy link
Copy Markdown
Member

Description

Pushdown sort aggregate metrics:

  • Add SORT_AGG_METRICS after AGGREGATION in pushdown context
  • Convert composite bucket agg to terms/histo/date-histo agg if sort one metric on single bucket
  • Convert composite bucket agg (all terms source) to multi-terms agg if sort one metric on multiple buckets

Related Issues

Resolves#4282

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>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin added the enhancement New feature or request label Oct 21, 2025
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin changed the title Pushdown sort aggregate metricsPushdown sort aggregate metrics for single bucketOct 24, 2025
return newTraitSet;
}

public CalciteLogicalIndexScan pushDownSortAggregateMetrics(Sort sort) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Will it be better to put these checks in one place if possible? Now the prerequisites are split both here and in aggAction.pushDownSortAggMetrics, although they are both aggregationBuilders and some checking seems overlapped.

@LantaoJinLantaoJin changed the title Pushdown sort aggregate metrics for single bucketPushdown sort aggregate metricsOct 24, 2025
// count agg optimized, get the path name from field names
path = fieldNames.get(collations.get(0).getFieldIndex());
} else if (metric instanceof ValuesSourceAggregationBuilder.LeafOnly) {
path = metric.getName();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit, non-blocking] Could this be combined with above branch? i.e. Does path = fieldNames.get(collations.get(0).getFieldIndex()); also works for metric instanceof ValuesSourceAggregationBuilder.LeafOnly?

List<String> fieldNames,
CompositeAggregationBuilder composite) {
String path;
AggregationBuilder metric = composite.getSubAggregations().stream().findFirst().orElse(null);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we only support only 1 agg metric for this enhancement? And anywhere to check this?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catching. added restricted checking.

Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJin
LantaoJin merged commit 5ebed84 into opensearch-project:mainOct 24, 2025
35 checks passed
@opensearch-trigger-bot

Copy link
Copy Markdown
Contributor

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 repositorycd$(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 treepushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-4603-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 5ebed8465dc9d498602a25b802207d63ee5030c8
# Push it to GitHub
git push --set-upstream origin backport/backport-4603-to-2.19-dev
# Go back to the original working treepopd# 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-4603-to-2.19-dev.

LantaoJin added a commit to LantaoJin/search-plugins-sql that referenced this pull request Oct 24, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
@LantaoJinLantaoJin added the backport-manually Filed a PR to backport manually. label Oct 24, 2025
yuancu pushed a commit that referenced this pull request Oct 27, 2025
* Pushdown sort aggregate metrics (#4603)
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
# Conflicts:
#	docs/user/ppl/functions/conversion.rst
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
Signed-off-by: Asif Bashar <asif.bashar@gmail.com>
expani pushed a commit to vinaykpud/sql that referenced this pull request Nov 4, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19-devbackport-failedbackport-manuallyFiled a PR to backport manually.enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Push down bucket aggregation with sorting by metrics to multi-terms aggregation

3 participants

@LantaoJin@yuancu@qianheng-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Pushdown sort aggregate metrics - #4603

Merged
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282
Oct 24, 2025
Merged

Pushdown sort aggregate metrics#4603
LantaoJin merged 14 commits into
opensearch-project:mainfrom
LantaoJin:pr/issues/4282

Conversation

@LantaoJin

@LantaoJinLantaoJin commented Oct 20, 2025

Copy link
Copy Markdown
Member

Description

Pushdown sort aggregate metrics:

  • Add SORT_AGG_METRICS after AGGREGATION in pushdown context
  • Convert composite bucket agg to terms/histo/date-histo agg if sort one metric on single bucket
  • Convert composite bucket agg (all terms source) to multi-terms agg if sort one metric on multiple buckets

Related Issues

Resolves#4282

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>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin added the enhancement New feature or request label Oct 21, 2025
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJinLantaoJin changed the title Pushdown sort aggregate metricsPushdown sort aggregate metrics for single bucketOct 24, 2025
return newTraitSet;
}

public CalciteLogicalIndexScan pushDownSortAggregateMetrics(Sort sort) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Will it be better to put these checks in one place if possible? Now the prerequisites are split both here and in aggAction.pushDownSortAggMetrics, although they are both aggregationBuilders and some checking seems overlapped.

@LantaoJinLantaoJin changed the title Pushdown sort aggregate metrics for single bucketPushdown sort aggregate metricsOct 24, 2025
// count agg optimized, get the path name from field names
path = fieldNames.get(collations.get(0).getFieldIndex());
} else if (metric instanceof ValuesSourceAggregationBuilder.LeafOnly) {
path = metric.getName();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit, non-blocking] Could this be combined with above branch? i.e. Does path = fieldNames.get(collations.get(0).getFieldIndex()); also works for metric instanceof ValuesSourceAggregationBuilder.LeafOnly?

List<String> fieldNames,
CompositeAggregationBuilder composite) {
String path;
AggregationBuilder metric = composite.getSubAggregations().stream().findFirst().orElse(null);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we only support only 1 agg metric for this enhancement? And anywhere to check this?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catching. added restricted checking.

Signed-off-by: Lantao Jin <ltjin@amazon.com>
@LantaoJin
LantaoJin merged commit 5ebed84 into opensearch-project:mainOct 24, 2025
35 checks passed
@opensearch-trigger-bot

Copy link
Copy Markdown
Contributor

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 repositorycd$(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 treepushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-4603-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 5ebed8465dc9d498602a25b802207d63ee5030c8
# Push it to GitHub
git push --set-upstream origin backport/backport-4603-to-2.19-dev
# Go back to the original working treepopd# 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-4603-to-2.19-dev.

LantaoJin added a commit to LantaoJin/search-plugins-sql that referenced this pull request Oct 24, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
@LantaoJinLantaoJin added the backport-manually Filed a PR to backport manually. label Oct 24, 2025
yuancu pushed a commit that referenced this pull request Oct 27, 2025
* Pushdown sort aggregate metrics (#4603)
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
(cherry picked from commit 5ebed84)
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
# Conflicts:
#	docs/user/ppl/functions/conversion.rst
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
Make nested alias type support referring to outer context (opensearch-project#4673)
Update big5 ppl queries and check plans (opensearch-project#4668)
Support push down sort after limit (opensearch-project#4657)
Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
Fix bin nested fields issue (opensearch-project#4606)
Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
Pushdown sort aggregate metrics (opensearch-project#4603)
Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
Allow renaming group-by fields to existing field names (opensearch-project#4586)
Publish internal modules separately for downstream reuse (opensearch-project#4484)
Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
Pushdown case function in aggregations as range queries (opensearch-project#4400)
Update GEOIP function to support IP types as input (opensearch-project#4613)
...
Signed-off-by: Asif Bashar <asif.bashar@gmail.com>
expani pushed a commit to vinaykpud/sql that referenced this pull request Nov 4, 2025
* convert sort aggregate metrics to term sort
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* refactor
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* fix conflicts2
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* Add more javadoc
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* address comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* delete incorrect comments
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* convert composite agg to multi-terms agg for sort metrics on multiple buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
* avoid the case of 'stat count, sum ... | sort count'
Signed-off-by: Lantao Jin <ltjin@amazon.com>
---------
Signed-off-by: Lantao Jin <ltjin@amazon.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19-devbackport-failedbackport-manuallyFiled a PR to backport manually.enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Push down bucket aggregation with sorting by metrics to multi-terms aggregation

3 participants

@LantaoJin@yuancu@qianheng-aws