-
Notifications
You must be signed in to change notification settings - Fork 181
Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations #4621
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@ritvibhatt Please take a look at the changes in |
Swiddis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall lgtm
| * @param hit the SearchHit to extract value from | ||
| * @return the extracted value, or null if no value found | ||
| */ | ||
| public static Object extractValue(SearchHit hit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Return Optional<Object> for type safety
docs/user/ppl/cmd/stats.rst
Outdated
| | cnt | take(email, 5) | age_span | gender | | ||
| |-----+--------------------------------------------+----------+--------| | ||
| | 1 | [] | 25 | F | | ||
| | 1 | [null] | 25 | F | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this expected? As discussed, shall we keep previous semantic of each function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to avoid breaking change
integ-test/src/test/resources/expectedOutput/calcite/explain_earliest_latest.json
Outdated
Show resolved
Hide resolved
opensearch/src/main/java/org/opensearch/sql/opensearch/response/agg/TopHitsParser.java
Outdated
Show resolved
Hide resolved
…ions Signed-off-by: Kai Huang <ahkcs@amazon.com>
| } else { | ||
| yield Pair.of( | ||
| AggregationBuilders.topHits(aggFieldName) | ||
| .fetchSource(helper.inferNamedField(args.getFirst()).getRootName(), null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should prefer fetch fields API. This will break for certain field types, e.g., text, nested. Please double check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to use fetch fields API
Signed-off-by: Kai Huang <ahkcs@amazon.com>
|
The backport to 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-4621-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c6a3a70ac0d0e3ae3aa5eaefba88dab000bc70d5
# Push it to GitHub
git push --set-upstream origin backport/backport-4621-to-2.19-dev
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/sql/backport-2.19-devThen, create a pull request where the |
…ions (opensearch-project#4621) (cherry picked from commit c6a3a70) Signed-off-by: Kai Huang <ahkcs@amazon.com>
* 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
* 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>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Simeon Widdis <sawiddis@amazon.com> Co-authored-by: Manasvini B S <manasvis@amazon.com> Co-authored-by: opensearch-ci-bot <opensearch-infra@amazon.com> Co-authored-by: Louis Chu <clingzhi@amazon.com> Co-authored-by: Chen Dai <daichen@amazon.com> Co-authored-by: Mebsina <cnoramut@gmail.com> Co-authored-by: Yuanchun Shen <yuanchu@amazon.com> Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Co-authored-by: Kai Huang <105710027+ahkcs@users.noreply.github.com> Co-authored-by: Peng Huo <penghuo@gmail.com> Co-authored-by: Alexey Temnikov <alexey.temnikov@improving.com> Co-authored-by: Riley Jerger <214163063+RileyJergerAmazon@users.noreply.github.com> Co-authored-by: Tomoyuki MORITA <moritato@amazon.com> Co-authored-by: Lantao Jin <ltjin@amazon.com> Co-authored-by: Songkan Tang <songkant@amazon.com> Co-authored-by: qianheng <qianheng@amazon.com> Co-authored-by: Simeon Widdis <sawiddis@gmail.com> Co-authored-by: Xinyuan Lu <xinyual@amazon.com> Co-authored-by: Jialiang Liang <jiallian@amazon.com> Co-authored-by: Peter Zhu <zhujiaxi@amazon.com> Co-authored-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com> Co-authored-by: expani <anijainc@amazon.com> Co-authored-by: expani1729 <110471048+expani@users.noreply.github.com> Co-authored-by: Vamsi Manohar <reddyvam@amazon.com> Co-authored-by: ritvibhatt <53196324+ritvibhatt@users.noreply.github.com> Co-authored-by: Xinyu Hao <75524174+ishaoxy@users.noreply.github.com> Co-authored-by: Marc Handalian <marc.handalian@gmail.com> Co-authored-by: Marc Handalian <handalm@amazon.com> 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)
Description
MIN, MAX, FIRST, LAST, and TAKE aggregations returned null or empty results when used with alias fields (e.g., @timestamp aliasing created_at) because they used .fetchSource() which cannot access alias fields.
Changes:
Related Issues
MIN,MAX,FIRST,LAST,TAKE) Return Null/Empty Values With Alias Fields in PPL #4595