zhuqi-lucas commented on PR #25050: URL: https://github.com/apache/datafusion/pull/25050#issuecomment-5594487340
Thanks @comphead — all five addressed; per-item notes: 1. **Tie-break**: documented as a user-visible behavior change in the PR description (new section) and pinned by a tied-keys case in `first_last_ordered.slt`. Release-notes line suggested there too. 2. **is_single gate coverage**: added a Boolean ROLLUP slt case (Boolean is outside `groups_accumulator_supported`, so it exercises the `GroupsAccumulatorAdapter` path that consumed the flag on `main`), plus two rule-level unit tests in `core/tests/physical_optimizer/update_aggr_exprs.rs` asserting the flag is set for a single grouping set and not set under grouping sets. Kept in this PR for now — happy to split it out for backporting if maintainers prefer. 3. **O(rows) winners**: `update_batch_pre_ordered` now records touched groups on the scoreboard's false-to-true transition and visits/clears only those. One wrinkle found while doing it: spill replay calls `merge_batch` on the same accumulator instance and the tournament helper leaves its winners' bits set, so a dirty flag triggers one full reset in that case — covered by a merge-interleave test and a sparse-1M-groups test. 4. **Benches**: split into a separate bench-only PR (coming next) so before/after numbers can be taken on main first. 5. **Fuzzer**: first/last ORDER BYs are now drawn from a prefix of the dataset's sort keys half the time, so the sorted datasets exercise the fast path while the unsorted dataset runs the comparing path on the same query. To keep the comparison exact despite item 1, the aggregate argument is the last ORDER BY column: rows tying on the whole ORDER BY carry equal output values, so the tie-break difference cannot produce false mismatches. `test_first_val`/`test_last_val` pass with the new mode. Local runs: aggregate crate 218 unit tests, rule tests, `first_last_ordered.slt`, and the full sqllogictest suite are green. -- 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]
