mrhhsg commented on code in PR #68651:
URL: https://github.com/apache/doris/pull/68651#discussion_r4153001563
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulator.java:
##########
@@ -210,6 +219,55 @@ private boolean
onePhaseAggWithDistribute(PhysicalHashAggregate<? extends Plan>
&& children.get(0).getPlan() instanceof PhysicalDistribute;
}
+ /**
+ * Check data-volume gates for bucketed hash aggregation using group-level
+ * statistics available during property regulation. Returns true if the
+ * pattern should be allowed (stats pass or unavailable), false if it
should
+ * be banned due to unfavorable data characteristics.
+ * Mirrors the checks from the old implementBucketedPhase.
+ */
+ private boolean bucketedDataVolumeGatesPass(PhysicalHashAggregate<?
extends Plan> aggregate) {
+ Statistics inputStats =
aggregate.getGroupExpression().get().childStatistics(0);
+ if (inputStats == null) {
+ return true; // no stats → allow (other gates handle eligibility)
+ }
+ Statistics outputStats = aggregate.getGroupExpression().get()
+ .getOwnerGroup().getStatistics();
+ SessionVariable sv = ConnectContext.get().getSessionVariable();
+ double rows = inputStats.getRowCount();
+
+ // Gate 1: minimum input rows
+ if (sv.bucketedAggMinInputRows > 0 && rows <
sv.bucketedAggMinInputRows) {
+ return false;
+ }
+
+ // Gate 2: high-cardinality GROUP BY columns
+ double highCardThreshold = sv.bucketedAggHighCardThreshold;
+ if (highCardThreshold > 0) {
+ for (Expression groupByKey : aggregate.getGroupByExpressions()) {
+ ColumnStatistic colStat =
inputStats.findColumnStatistics(groupByKey);
+ if (colStat != null && !colStat.isUnKnown
+ && colStat.ndv > rows * highCardThreshold) {
+ return false;
+ }
+ }
+ }
+
+ // Gate 3: max group keys (merge phase cost dominates)
+ if (sv.bucketedAggMaxGroupKeys > 0 && outputStats != null
+ && outputStats.getRowCount() > sv.bucketedAggMaxGroupKeys) {
+ return false;
+ }
+
+ // Gate 4: aggregation output cardinality ratio
Review Comment:
Follow-up: the change made for this thread (6a19a235100, skipping the
output-ratio gate when the group key statistics are unknown) is reverted in
08e64c7f52d, so this gate is identical to master again.
It turned out to be the cause of the random explain-shape failures in the P0
/ Cloud P0 pipelines of this PR. The regression FEs run with
`use_fuzzy_session_variable=true`, and `initFuzzyModeVariables()` sets
`bucketed_agg_min_input_rows` to 0 for about half of the connections (that came
with #61495 and is the same on master). On master this is harmless for
un-analyzed tables because the output-ratio gate rejects the `rows / 3`
fallback. With the gate skipped, a small un-analyzed table had no data-volume
gate left on those connections, so its plan depended on which connection the
suite got: a different subset of shape suites lost `hashAgg[LOCAL]` in every
run, and in `nereids_tpch_p0/tpch/topn-filter` the aggregate below the join
became a one-phase aggregate over a raw-row exchange (`TOPN OPT:6` instead of
`TOPN OPT:7`).
The point raised here is still valid as a limitation — with the default
thresholds an un-analyzed table never gets the bucketed plan — but it is shared
with master and has to be addressed there first, together with the fuzzy value
and the shape expectations, and then picked.
`BucketedAggregateTranslatorTest#testUnknownGroupKeyStatisticsKeepRegularAggregationByDefault`
now pins the master behaviour (two-phase with the default threshold, fused
once the threshold admits the fallback estimate).
--
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]
