xiangfu0 commented on code in PR #19800:
URL: https://github.com/apache/pinot/pull/19800#discussion_r4224469629
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MultistageGroupByExecutor.java:
##########
@@ -230,6 +291,30 @@ public List<Object[]> getResult(Comparator<Object[]>
comparator, int maxRows) {
return result;
}
+ private SortedRow getSortedRow(Object[] row, int[] columnsToFinalize) {
+ if (columnsToFinalize.length == 0) {
+ return new SortedRow(row, row);
+ }
+ // Extract each sort value once per group, retaining intermediate values
for downstream aggregation.
+ Object[] sortKeys = row.clone();
+ for (int index : columnsToFinalize) {
+ AggregationFunction aggFunction = _aggFunctions[index -
_groupKeyIds.length];
+ Object intermediateResult = row[index];
+ // Funnel finalization consumes the event queue, so extract from a copy.
+ if (intermediateResult instanceof PriorityQueue<?> queue) {
+ intermediateResult = new PriorityQueue<>(queue);
+ }
+ Object finalResult = aggFunction.extractFinalResult(intermediateResult);
+ sortKeys[index] = finalResult != null
Review Comment:
**Wrong trim ordering for SUMPRECISION (regression).** The sort key is
converted with `aggFunction.getFinalResultColumnType().getStoredType()` — the
SSE wire type — not the type the MSE FINAL stage emits.
`SumPrecisionAggregationFunction.getFinalResultColumnType()` returns `STRING`
(with a TODO), so `TypeUtils.convert` turns each `BigDecimal` sum into
`toString()` and the trim heap orders groups lexicographically: `"9" > "2500" >
"100" > "10"`, and scientific notation like `1E+3` misorders further. Before
this PR the heap compared the `BigDecimal` intermediates numerically (they are
`Comparable`), which was correct; MSE FINAL/DIRECT and SSE `TableResizer` also
compare numerically. With group trim on a LEAF above a join and `ORDER BY
SUMPRECISION(...) DESC LIMIT 1` over sums {5..9, 100}, the 100-group is trimmed
and the query silently returns 9. Same root cause gives subtler divergence for
any function whose declared final type disagrees with the planner type (e.g.
`PERCENTILES
MARTTDIGEST` pinned to ARG0 truncates the double before comparing). The sort
key should be built in the type the downstream sort actually compares (the
planned output type), or such columns should keep their natural `Comparable`
intermediate ordering.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
