Baymine opened a new pull request, #66045:
URL: https://github.com/apache/doris/pull/66045
### What problem does this PR solve?
Issue Number: close #66044
Problem Summary:
The `MultiDistinct` strategy keeps a per-group hash set of every distinct
value on a single node.
For a group-by query whose distinct argument is near-unique (or has unknown
statistics that could
be near-unique), one node can hold a whole group's value set and OOM. The
no-group-by branch of
`DistinctAggStrategySelector.shouldUseMultiDistinct` already guards against
this; the group-by
branch did not.
This PR adds two coordinated guards to the group-by branch of
`shouldUseMultiDistinct`, matching
the no-group-by branch's behavior (two commits, logically distinct):
1. `hasHighNdvDistinctArgument`: force CTE split when any distinct
argument's estimated NDV >=
`row * MID_CARDINALITY_THRESHOLD`. Estimates via `ExpressionEstimation`
so `if(cond, col, null)`
propagates `col`'s NDV. Placed before `hasUnknownStatistics(groupBy)` so
a function group-by key
with unknown stats is still protected. Unknown stats do not trigger this
guard (avoids pushing
safe cases onto the costlier CTE path).
2. `hasUnknownNdvDistinctArgument`: force CTE split when a distinct
argument's estimate is unknown
OR any input slot it reads has absent/unknown statistics. The per-slot
scan is required because
`ExpressionEstimation.visitIf` fabricates a small non-unknown NDV for
`if(cond, col, null)` even
when `col` is unanalyzed, which would otherwise let a near-unique wrapped
column escape guard #1.
3. When group-by stats are unknown (now reached only when every distinct
argument is confirmed
low-NDV), prefer CTE split for a single group-by key (parallelism is
capped at the unknown group
count) and keep `MultiDistinct` for >= 2 keys (joint cardinality is
usually high enough to spread
safely).
### Release note
MultiDistinct is no longer chosen for group-by queries whose distinct
argument is near-unique
relative to input row count, whose distinct argument has unknown statistics
(including arguments
wrapped in expressions such as `if(...)` whose underlying input slots are
unanalyzed), or whose
single group-by key has unknown statistics; such queries route to the CTE
split strategy to avoid
potential OOM.
### Check List (For Author)
- Test: Unit Test + Regression test
- FE unit test `SplitMultiDistinctTest#multiSumWithGby`: updated to
assert the CTE split shape.
- FE unit test `AggregateUtilsHighNdvDistinctTest` (new, 11 cases):
covers both helpers —
high-NDV triggers / low-NDV doesn't / unknown doesn't trigger the
high-NDV guard / if-wrapped
near-unique triggers / threshold boundary; and for the unknown guard:
all-unknown triggers /
one-unknown-among-known triggers / all-known doesn't /
if-wrapped-known doesn't /
if-wrapped-unknown triggers (the per-slot-scan gap) /
year()-over-unknown-slot triggers.
- Regression test `distinct_agg_strategy_selector`: the pre-existing
WITH-stats case
`should_use_multi_distinct_with_group_by` (query `count(distinct
d_20), count(distinct b_5)
... group by a_1`, where `d_20` is near-unique) now correctly routes
to CTE split, so it was
renamed to `should_use_cte_with_group_by_high_ndv` with an explanatory
comment; and the
no-stats case was renamed `no_stats_should_use_multi_distinct` ->
`no_stats_should_use_cte_with_group_by`.
The `.out` was regenerated by the regression harness against a live
cluster (not hand-written).
- Behavior changed: Yes (multi-distinct strategy selection for group-by
queries with high-NDV or
unknown-statistics distinct arguments, or unknown single-key group-by
stats, now prefers CTE split).
- Does this need documentation: No
--
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]
