mohitgurav20 commented on code in PR #26000:
URL: https://github.com/apache/datafusion/pull/26000#discussion_r4571969303
##########
datafusion/common/src/config.rs:
##########
@@ -1940,16 +1940,25 @@ config_namespace! {
/// projections or aliases.
pub enable_unions_to_filter: bool, default = false
- /// Prefer native grouped DISTINCT aggregation when target_partitions
is 1
- /// and every aggregate explicitly supports a specialized
GroupsAccumulator
- /// for its DISTINCT argument types. Defaults to false, preserving the
- /// two-phase GROUP BY rewrite.
+ /// When `true`, the `SingleDistinctToGroupBy` optimizer rule keeps a
grouped
+ /// DISTINCT aggregate native, instead of rewriting it into a
two-phase `GROUP BY`,
+ /// when every DISTINCT argument is a fixed-width, non-float type.
This lowers peak
+ /// memory when no memory limit is set, because the intermediate
+ /// `(group, distinct value)` result is never built.
///
- /// The native plan can avoid materializing distinct (group, value)
pairs,
- /// but can be slower for many groups with duplicate values. It must
fit
- /// each group's distinct values in memory, so queries that succeed by
- /// spilling with the rewrite may fail with this option enabled. One
target
- /// partition can still have multiple input partitions and merge
native states.
+ /// The native plan avoids materializing intermediate `(group,
distinct value)`
+ /// pairs, which lowers peak memory when there are few groups with many
+ /// distinct values each. With many small groups it can use more
memory and
+ /// run slower, because functions without a specialized DISTINCT
+ /// `GroupsAccumulator` (such as `sum`) keep one accumulator per group.
+ /// Native DISTINCT must also fit each group's distinct values in
memory:
+ /// under a memory limit, the rewrite can spill those pairs, but the
native
+ /// accumulator cannot, so queries that succeed by spilling with the
rewrite
+ /// may fail with this option enabled.
+ ///
+ /// Queries without `GROUP BY`, and other argument types, keep the
rewrite.
Review Comment:
@jayzhan211 sirr i Applied the updated doc wording across config.rs,
configs.md, and information_schema.slt. Verified information_schema.slt and the
Prettier checks pass cleanly. Thanks for catching the spill distinction.
--
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]
