Skip to content

Conversation

@ahkcs
Copy link
Contributor

@ahkcs ahkcs commented Oct 21, 2025

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:

  - Aggregation builders: Changed from .fetchSource() to .fetchField() to use OpenSearch's fields API
  - Response parsers: Enhanced TopHitsParser and ArgMaxMinParser to extract values from hit.getFields()
  # Before: Returns null
  source=index | stats MIN(@timestamp), MAX(@timestamp)

  # After: Returns correct min/max values
  {"MIN(@timestamp)": "2024-01-01T10:00:00.000Z", "MAX(@timestamp)": "2024-01-03T10:00:00.000Z"}

Related Issues

@ahkcs
Copy link
Contributor Author

ahkcs commented Oct 21, 2025

@ritvibhatt Please take a look at the changes in ArgMaxMinParser to see if makes sense to you

@dai-chen dai-chen added PPL Piped processing language backport 2.19-dev bug Something isn't working labels Oct 22, 2025
Swiddis
Swiddis previously approved these changes Oct 22, 2025
Copy link
Collaborator

@Swiddis Swiddis left a 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) {
Copy link
Collaborator

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

| cnt | take(email, 5) | age_span | gender |
|-----+--------------------------------------------+----------+--------|
| 1 | [] | 25 | F |
| 1 | [null] | 25 | F |
Copy link
Collaborator

@dai-chen dai-chen Oct 22, 2025

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?

Copy link
Contributor Author

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

Signed-off-by: Kai Huang <[email protected]>
Swiddis
Swiddis previously approved these changes Oct 24, 2025
} else {
yield Pair.of(
AggregationBuilders.topHits(aggFieldName)
.fetchSource(helper.inferNamedField(args.getFirst()).getRootName(), null)
Copy link
Collaborator

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.

Copy link
Contributor Author

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 <[email protected]>
@Swiddis Swiddis merged commit c6a3a70 into opensearch-project:main Oct 24, 2025
32 of 33 checks passed
@opensearch-trigger-bot
Copy link
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 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-dev

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

ahkcs added a commit to ahkcs/sql that referenced this pull request Oct 27, 2025
@LantaoJin LantaoJin added the backport-manually Filed a PR to backport manually. label Oct 27, 2025
RyanL1997 pushed a commit that referenced this pull request Oct 27, 2025
…T, and TAKE Aggregations (#4669)

Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (#4621)
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 <[email protected]>
expani pushed a commit to vinaykpud/sql that referenced this pull request Nov 4, 2025
sandeshkr419 added a commit to sandeshkr419/sql that referenced this pull request Dec 3, 2025
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19-dev backport-failed backport-manually Filed a PR to backport manually. bug Something isn't working PPL Piped processing language

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Multiple Aggregation Functions (MIN, MAX, FIRST, LAST, TAKE) Return Null/Empty Values With Alias Fields in PPL

4 participants