Fix mapping after aggregation push down #4500

qianheng-aws · 2025-10-10T08:55:56Z

Description

Allow type updating/overriding when pushing aggregation and it has derived fields with the same name as fields in source.

Also fix WidthBucketFunction to return correct type if the field is type of TIME/DATE/TIMESTAMP.

Related Issues

Resolves #4115, #3458

Check List

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

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

Signed-off-by: Heng Qian <[email protected]>

LantaoJin · 2025-10-11T02:11:56Z

can you update the resolve # section? why resolve a merged pr? you mean a followup?

LantaoJin · 2025-10-11T02:14:47Z

integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/4415.yml

+          query: source=test | bin num bins=3 | stats count() by num
+
+  - match: { total: 2 }
+  - match: {"datarows": [[2, "10-20"], [1, "20-30"]]}


what's the result without this patch? is it a test about overwrite existing type? if yes, better to match the schema too.

Corrected the issue link, #4115. It will throw exception without this PR

Also add the schema verification in the commit acecdd1

LantaoJin · 2025-10-11T02:16:46Z

...earch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java

-      // of all niceties of all types.
-      this.typeMapping.putIfAbsent(field, typeMapping.get(field));
-    }
+    this.typeMapping.putAll(typeMapping);


q: should we check Settings.Key.PPL_SYNTAX_LEGACY_PREFERRED first?

This is a bug actually. We'd better always have this change despite of what this configuration is.

LantaoJin · 2025-10-11T02:20:45Z

...h/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java

  // aggregation adds info about new columns to the factory,
-  // it is accepted without overwriting existing data.
-  public void factoryMappingsAreExtendableWithoutOverWrite()
+  // it will overwrite existing type to fix issue #4413.


what is the issue 4413? can you attach the link?

Signed-off-by: Heng Qian <[email protected]>

yuancu

LGTM. Can you link this PR to #3458 as well? It solves the mismatched type mapping when calcite is disabled.

opensearch-trigger-bot · 2025-10-13T02:43:04Z

The backport to 2.19-dev failed:

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

To backport manually, run these commands in your terminal:

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

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

* Fix mapping after aggregation push down Signed-off-by: Heng Qian <[email protected]> * Fix IT and UT Signed-off-by: Heng Qian <[email protected]> * address comments Signed-off-by: Heng Qian <[email protected]> --------- Signed-off-by: Heng Qian <[email protected]> (cherry picked from commit 9257889) Signed-off-by: Heng Qian <[email protected]>

) * Fix mapping after aggregation push down (#4500) * Fix mapping after aggregation push down Signed-off-by: Heng Qian <[email protected]> * Fix IT and UT Signed-off-by: Heng Qian <[email protected]> * address comments Signed-off-by: Heng Qian <[email protected]> --------- Signed-off-by: Heng Qian <[email protected]> (cherry picked from commit 9257889) Signed-off-by: Heng Qian <[email protected]> * Fix compiling Signed-off-by: Heng Qian <[email protected]> --------- Signed-off-by: Heng Qian <[email protected]>

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Simeon Widdis <[email protected]> Co-authored-by: Manasvini B S <[email protected]> Co-authored-by: opensearch-ci-bot <[email protected]> Co-authored-by: Louis Chu <[email protected]> Co-authored-by: Chen Dai <[email protected]> Co-authored-by: Mebsina <[email protected]> Co-authored-by: Yuanchun Shen <[email protected]> Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Co-authored-by: Kai Huang <[email protected]> Co-authored-by: Peng Huo <[email protected]> Co-authored-by: Alexey Temnikov <[email protected]> Co-authored-by: Riley Jerger <[email protected]> Co-authored-by: Tomoyuki MORITA <[email protected]> Co-authored-by: Lantao Jin <[email protected]> Co-authored-by: Songkan Tang <[email protected]> Co-authored-by: qianheng <[email protected]> Co-authored-by: Simeon Widdis <[email protected]> Co-authored-by: Xinyuan Lu <[email protected]> Co-authored-by: Jialiang Liang <[email protected]> Co-authored-by: Peter Zhu <[email protected]> Co-authored-by: Vinay Krishna Pudyodu <[email protected]> Co-authored-by: expani <[email protected]> Co-authored-by: expani1729 <[email protected]> Co-authored-by: Vamsi Manohar <[email protected]> Co-authored-by: ritvibhatt <[email protected]> Co-authored-by: Xinyu Hao <[email protected]> Co-authored-by: Marc Handalian <[email protected]> Co-authored-by: Marc Handalian <[email protected]> Fix join type ambiguous issue when specify the join type with sql-like join criteria (opensearch-project#4474) Fix issue 4441 (opensearch-project#4449) Fix missing keywordsCanBeId (opensearch-project#4491) Fix the bug of explicit makeNullLiteral for UDT fields (opensearch-project#4475) Fix mapping after aggregation push down (opensearch-project#4500) Fix percentile bug (opensearch-project#4539) Fix JsonExtractAllFunctionIT failure (opensearch-project#4556) Fix sort push down into agg after project already pushed (opensearch-project#4546) Fix push down failure for min/max on derived field (opensearch-project#4572) Fix compile issue in main (opensearch-project#4608) Fix filter parsing failure on date fields with non-default format (opensearch-project#4616) Fix bin nested fields issue (opensearch-project#4606) Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621) fix rename issue (opensearch-project#4670) Fixes for `Multisearch` and `Append` command (opensearch-project#4512) Fix asc/desc keyword behavior for sort command (opensearch-project#4651) Fix] Fix unexpected shift of extraction for `rex` with nested capture groups in named groups (opensearch-project#4641) Fix CVE-2025-48924 (opensearch-project#4665) Fix sub-fields accessing of generated structs (opensearch-project#4683) Fix] Incorrect Field Index Mapping in AVG to SUM/COUNT Conversion (opensearch-project#15)

qianheng-aws added 2 commits October 10, 2025 16:27

Fix mapping after aggregation push down

ea76d6d

Signed-off-by: Heng Qian <[email protected]>

Fix IT and UT

bdfc888

Signed-off-by: Heng Qian <[email protected]>

qianheng-aws added the bug Something isn't working label Oct 10, 2025

LantaoJin reviewed Oct 11, 2025

View reviewed changes

address comments

acecdd1

Signed-off-by: Heng Qian <[email protected]>

yuancu approved these changes Oct 11, 2025

View reviewed changes

qianheng-aws mentioned this pull request Oct 11, 2025

[BUG] PPL query fails when using existing field name in as clause #3458

Closed

LantaoJin approved these changes Oct 13, 2025

View reviewed changes

LantaoJin added the backport 2.19-dev label Oct 13, 2025

LantaoJin merged commit 9257889 into opensearch-project:main Oct 13, 2025
36 checks passed

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

qianheng-aws mentioned this pull request Oct 13, 2025

[Backport 2.19-dev] Fix mapping after aggregation push down #4500 #4524

Merged

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

qianheng-aws mentioned this pull request Oct 27, 2025

[BUG] stats command throw exception when alias of metrics with existing column name #4192

Closed

LantaoJin mentioned this pull request Nov 24, 2025

Grouping key field type can only be overwritten when the ExprCoreTypes are different #4850

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix mapping after aggregation push down #4500

Fix mapping after aggregation push down #4500

Uh oh!

qianheng-aws commented Oct 10, 2025 •

edited

Loading

Uh oh!

LantaoJin commented Oct 11, 2025

Uh oh!

LantaoJin Oct 11, 2025

Uh oh!

qianheng-aws Oct 11, 2025

Uh oh!

qianheng-aws Oct 11, 2025

Uh oh!

LantaoJin Oct 11, 2025

Uh oh!

qianheng-aws Oct 11, 2025

Uh oh!

LantaoJin Oct 11, 2025

Uh oh!

qianheng-aws Oct 11, 2025

Uh oh!

yuancu left a comment

Uh oh!

Uh oh!

opensearch-trigger-bot bot commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fix mapping after aggregation push down #4500

Fix mapping after aggregation push down #4500

Uh oh!

Conversation

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

Uh oh!

Description

Related Issues

Check List

Uh oh!

LantaoJin commented Oct 11, 2025

Uh oh!

LantaoJin Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

qianheng-aws Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

qianheng-aws Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

LantaoJin Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

qianheng-aws Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

LantaoJin Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

qianheng-aws Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

yuancu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

opensearch-trigger-bot bot commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

qianheng-aws commented Oct 10, 2025 •

edited

Loading