jayzhan211 commented on code in PR #26000:
URL: https://github.com/apache/datafusion/pull/26000#discussion_r4571608071
##########
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:
Optional cleanup: this doc no longer says the setting only applies at
`target_partitions = 1` with all-DISTINCT aggregates, so with default settings
turning it on does nothing. Its first paragraph also says memory always goes
down, while the second says many small groups need more. Last, "the native
accumulator cannot" spill was my wording and it's wrong: at one partition with
50k groups it spilled 12 times and matched the rewrite. What it can't do is
split one large group. This version fixes all three (I checked that
`dev/update_config_docs.sh` and `information_schema.slt` pass with it):
```suggestion
/// When `true` and `target_partitions` is 1, the
`SingleDistinctToGroupBy`
/// optimizer rule keeps a grouped aggregate whose aggregates are
all DISTINCT
/// native, instead of rewriting it into a two-phase `GROUP BY`,
when every
/// DISTINCT argument is an integer, boolean, decimal, date, time,
timestamp
/// or interval type.
///
/// 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, it can spill whole groups but not split
one group's
/// distinct values, so a query with a few large groups that
succeeds by
/// spilling with the rewrite may fail with this option enabled.
///
/// Queries without `GROUP BY`, and other argument types, keep the
rewrite.
```
--
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]
