QinXi-ai commented on issue #25936: URL: https://github.com/apache/datafusion/issues/25936#issuecomment-6721816577
Following up on [@alamb's comment on #25988](https://github.com/apache/datafusion/pull/25988#issuecomment-6721525769): I agree this issue should be reopened. The default still takes the original path. I should have removed "Closes #25936" when I changed the PR to opt-in. I've traced the spill path and have a plan for getting this enabled by default: 1. Start with the duplicate-heavy case from review. Keep the first non-null value for each group inline, and use the shared distinct hash set for additional values. This would avoid probing that set for every repeated value in Q10. I'll benchmark it against Q01, Q02 and Q10 to check the cost on other data shapes. 2. Add a spill path for single-stage integer `COUNT(DISTINCT)` that writes and merges `(group keys, distinct value)` pairs. The current path spills lists, sorts by group, then rebuilds each group's distinct set during replay. Sorting by the value too would let replay deduplicate and count incrementally, including when one group's values exceed memory. The existing spill-file and merge code should be reusable; draining values in bounded chunks and accounting for the conversion buffers need new work. 3. Move selection to the physical plan, initially covering a single integer distinct count with an actual single input partition. That would let us combine the existing deduplication and counting aggregates where the new path applies. Multiple input partitions would keep the rewrite initially. I'd use the existing setting for this and turn it on by default, with `false` retaining the rewrite as a fallback. So far I've checked the code and run a standalone Python model through 315 differential cases, including duplicates across spill runs, null-only groups and a group larger than the model's buffer. The Rust implementation and performance measurements are still ahead. I'll start with the accumulator change; the spill work is the larger part of making the default safe under a memory limit. -- 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]
