mrhhsg commented on code in PR #68651:
URL: https://github.com/apache/doris/pull/68651#discussion_r4140620417
##########
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:
Fixed in 6a19a235100.
Gate 4 of `bucketedDataVolumeGatesPass` now skips the output-ratio check
when any GROUP BY key has unknown statistics
(`AggregateUtils.hasUnknownStatistics`), which is exactly the case where
`StatsCalculator.estimateGroupByRowCount` returns the `rows *
DEFAULT_AGGREGATE_RATIO` placeholder instead of a real group cardinality. This
matches gate 2, which already ignores unknown NDVs, so the default
`bucketed_agg_high_card_threshold` (0.3) no longer bans every un-analyzed
table. When statistics are known the gate behaves as before. Gate 3
(`bucketed_agg_max_group_keys`, default 0 = off) is left unchanged since it
only applies when a user opts in.
Tests: FE UT
`BucketedAggregateTranslatorTest.testUnknownGroupKeyStatisticsKeepBucketedAggregation`
plans an un-analyzed table with the default threshold and expects
`BucketedAggregationNode` (it fails on the previous code), and the
`bucketed_hash_agg` regression suite gains Test 1b that keeps the default
threshold instead of raising it to 1.0. The 1.0 override remains for the other
cases because the tiny test tables would trip the NDV gate once analyzed; its
comment was rewritten accordingly.
--
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]