Fix wrong results for multi-key GROUP BY with transform expressions

[zeronerdzerogeekzerocool]

PR flow

Fix maps GROUP BY keys by full ExpressionContext and validates ORDER BY keys are present.

flowchart TD
  N0["getRecordKeyComparator #40;F1#41;"]:::stUnchanged
  N1["getGroupKeyOrderByExpressionFromRowOrderByExpressions #40;F1#41;"]:::stUnchanged
  N2["getGroupByExpressionIndexMap #40;F1#41;"]:::stModified
  N3["retrieve index for ORDER BY expr #40;F1#41;"]:::stModified
  N4["validate index not null #40;F1#41;"]:::stAdded
  N5["create OrderByExpressionWithIndex #40;F1#41;"]:::stUnchanged
  N6["return list of OrderByExpressionWithIndex #40;F1#41;"]:::stUnchanged
  N7["build comparator using indices #40;F1#41;"]:::stUnchanged
  N0 -->|"calls"| N1
  N1 -->|"calls"| N2
  N1 -->|"iterates and gets index"| N3
  N3 -->|"passes index to check"| N4
  N4 -->|"if not null#44; creates WithIndex"| N5
  N5 -->|"adds to result list"| N6
  N1 -->|"returns list after loop"| N6
  N0 -->|"uses list to build comparator"| N7
  classDef stAdded fill:#dafbe1,stroke:#1a7f37,color:#1f2328,stroke-width:2px
  classDef stModified fill:#fff8c5,stroke:#9a6700,color:#1f2328,stroke-width:2px
  classDef stRemoved fill:#ffebe9,stroke:#cf222e,color:#1f2328,stroke-width:2px
  classDef stUnchanged fill:#f6f8fa,stroke:#656d76,color:#1f2328,stroke-width:1px

Loading

AI-generated · Green: added · Yellow: modified · Red: removed · Gray: existing

Diff evidence

• F1: pinot-core/src/main/java/org/apache/pinot/core/query/utils/OrderByComparatorFactory.java — before · after

• Regenerate PR flow

OrderByComparatorFactory.getGroupByExpressionIndexMap keyed group-by expressions by expression.getIdentifier(), which is null for anything that isn't a plain column reference (e.g. dateTrunc(...), json_extract_index(...)). With two or more transform-based GROUP BY keys, every expression collapsed onto the same null map entry, so getRecordKeyComparator resolved every ORDER BY expression to the same column index instead of its own.

This corrupted the SSE sort-aggregate combine (SortedGroupByCombineOperator / SequentialSortedGroupByCombineOperator): when merging per-segment sorted group-by results, the comparator no longer distinguished between distinct group keys, so unequal groups could be treated as equal and merged, silently corrupting aggregated values for multi-key GROUP BY queries where at least two keys are transform expressions.

Fix: key the map by the whole ExpressionContext instead of its identifier, and fail fast via Preconditions.checkState if an ORDER BY expression isn't actually a GROUP BY key (rather than silently mapping to a wrong/null index).

Instructions:

1. The PR has to be tagged with at least one of the following labels (*):
   1. feature
   2. bugfix
   3. performance
   4. ui
   5. backward-incompat
   6. release-notes (**)
2. Remove these instructions before publishing the PR.

(*) Other labels to consider:

• testing
• dependencies
• docker
• kubernetes
• observability
• security
• code-style
• extension-point
• refactor
• cleanup

(**) Use release-notes label for scenarios like:

• New configuration options
• Deprecation of configurations
• Signature changes to public methods/interfaces
• New plugins added or old plugins removed

[codecov-commenter]

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.98%. Comparing base (bd1e882) to head (4f91189).

Files with missing lines	Patch %	Lines
...not/core/query/utils/OrderByComparatorFactory.java	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #19663      +/-   ##
============================================
- Coverage     67.99%   67.98%   -0.01%     
  Complexity     1450     1450              
============================================
  Files          3511     3511              
  Lines        227483   227485       +2     
  Branches      35994    35995       +1     
============================================
- Hits         154676   154656      -20     
- Misses        60661    60681      +20     
- Partials      12146    12148       +2     

Flag	Coverage Δ
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (?)
java-25	67.98% <87.50%> (-0.01%)	⬇️
lane-a	100.00% <ø> (ø)
lane-b	0.00% <ø> (ø)
temurin	67.98% <87.50%> (-0.01%)	⬇️
unittests	67.98% <87.50%> (-0.01%)	⬇️
unittests1	58.18% <87.50%> (-0.01%)	⬇️
unittests2	39.64% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.