zhuqi-lucas commented on PR #25050: URL: https://github.com/apache/datafusion/pull/25050#issuecomment-5586119695
Thank you @jayzhan211 — confirmed, and great catch. I reproduced your ROLLUP example on this branch before touching anything (wrong results exactly as you showed), then applied the fix in `OptimizeAggregateOrder` as you suggested: the group-by prefix is only used for a single grouping set, so with grouping sets the aggregate's own ORDER BY must be satisfied on its own. Your repro now returns the expected rows. Added two cases to `first_last_ordered.slt`: - your ROLLUP repro (comparisons required, correct results), and - a ROLLUP where the aggregate ORDER BY leads the input ordering, which stays on the fast path since a globally sorted input is sorted within every group of every grouping set. Also worth noting for reviewers: as you said, the single-group accumulator consumed this flag through `GroupsAccumulatorAdapter` on `main` already, so the rule-level guard fixes that latent path too, not just the new grouped fast path. Full sqllogictest suite (511 files) and the aggregate/physical-optimizer unit tests pass locally with the change. -- 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]
