github-actions[bot] commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4366249927
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java:
##########
@@ -505,45 +512,139 @@ private Expression rewriteShuttledExpression(StructInfo
queryStructInfo, Express
* select b, count(*) from t1 group by b, after rewritten by materialized
view successfully
* If mv part partition is invalid, can not compensate union all, because
result is wrong after
* compensate union all.
+ *
+ * <p>The compensation unions two finished aggregates: what the
materialized view answers for the partitions
+ * it keeps, and what the base table answers for the partitions it does
not. It is right while no group is
+ * answerable by both, and only a grouping that reaches the column the
view is partitioned on keeps the two
+ * apart -- the view's partitions are disjoint in that column, by value
for a list partitioned view and by
+ * range for a range partitioned one. Grouping by an expression over it
does not: {@code date_trunc(d,
+ * 'year')} answers one year out of both branches, the view's part of it
and the base table's, and the two
+ * rows come out beside each other. Nor does a grouping set that leaves it
out -- the {@code ()} total of
+ * {@code ROLLUP}/{@code CUBE}/{@code GROUPING SETS} puts every partition
value in one group, which both
+ * branches answer.
*/
@Override
protected boolean canUnionRewrite(Plan queryPlan,
AsyncMaterializationContext context,
CascadesContext cascadesContext) {
- // Check query plan is contain the partition column
- // Query plan in the current rule must contain aggregate node, because
the rule pattern is
+ // The union is built over the whole query plan, so the outermost
aggregate is the one whose groups
+ // the two branches answer. The plan holds one, since the rule's
pattern requires it.
Optional<LogicalAggregate<Plan>> logicalAggregateOptional =
queryPlan.collectFirst(planTreeNode -> planTreeNode instanceof
LogicalAggregate);
if (!logicalAggregateOptional.isPresent()) {
return true;
}
- List<Expression> groupByExpressions =
logicalAggregateOptional.get().getGroupByExpressions();
+ LogicalAggregate<Plan> queryAggregate = logicalAggregateOptional.get();
+ List<Expression> groupByExpressions =
queryAggregate.getGroupByExpressions();
if (groupByExpressions.isEmpty()) {
// Scalar aggregate can not compensate union all
return false;
}
- MTMVPartitionInfo mvPartitionInfo =
context.getMtmv().getMvPartitionInfo();
- List<BaseColInfo> pctInfos = mvPartitionInfo.getPctInfos();
- boolean canUnionRewrite = false;
- // Check the query plan group by expression contains partition col or
not
- List<? extends Expression> groupByShuttledExpressions =
-
ExpressionUtils.shuttleExpressionWithLineage(groupByExpressions, queryPlan);
- for (Expression expression : groupByShuttledExpressions) {
- canUnionRewrite = !expression.collectToSet(expr -> {
- if (!(expr instanceof SlotReference) || !((SlotReference)
expr).isColumnFromTable()) {
- return false;
+ List<BaseColInfo> pctInfos =
context.getMtmv().getMvPartitionInfo().getPctInfos();
+ // The view has one partition column but it can be tracked by several
base columns at once: an inner
+ // join on A.d = B.d records both, and they are one dimension as long
as the query holds them equal.
+ Map<BaseColInfo, Set<BaseColInfo>> equalColumns =
columnsTheQueryHoldsEqual(queryPlan);
+ Optional<LogicalRepeat<?>> logicalRepeatOptional =
queryAggregate.getSourceRepeat();
+ if (!logicalRepeatOptional.isPresent()) {
+ return groupingCoversPartitionColumns(groupByExpressions,
queryPlan, pctInfos, equalColumns);
+ }
+ // The group by list holds every grouping set's expressions beside the
grouping id, so it can not tell
+ // which set a partition column was read from; each set is asked on
its own instead.
+ for (List<Expression> groupingSet :
logicalRepeatOptional.get().getGroupingSets()) {
+ if (!groupingCoversPartitionColumns(groupingSet, queryPlan,
pctInfos, equalColumns)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Whether these grouping expressions reach every column the view is
partitioned on, each of them by itself
+ * or equal to one that is -- through another column too, since an
equality chain is one equality. An
+ * expression over a partition column puts several of its values in one
group, and a tracked column that is
+ * neither grouped nor tied to a grouped one by the query varies freely
within a group.
+ */
+ private static boolean groupingCoversPartitionColumns(List<Expression>
groupingExpressions, Plan queryPlan,
+ List<BaseColInfo> pctInfos, Map<BaseColInfo, Set<BaseColInfo>>
equalColumns) {
+ List<? extends Expression> groupingShuttledExpressions =
+
ExpressionUtils.shuttleExpressionWithLineage(groupingExpressions, queryPlan);
+ Set<BaseColInfo> partitionColumns = Sets.newHashSet(pctInfos);
+ Set<BaseColInfo> groupedTogether = Sets.newHashSet();
+ Deque<BaseColInfo> unvisited = new ArrayDeque<>();
+ for (Expression expression : groupingShuttledExpressions) {
+ Optional<BaseColInfo> grouped = columnGroupedByItself(expression);
Review Comment:
[P2] Keep unions grouped by the MV partition expression
The bare-slot requirement also rejects grouping by the MV's own RANGE
partition transform. Use populated daily `RANGE(d)` partitions for Jan 1, Feb 1
and an unrelated Apr 1; refresh `PARTITION BY(date_trunc(d,'month')) AS SELECT
d,SUM(v) s FROM t GROUP BY d`, write Jan 1, then query Jan+Feb grouped only by
`date_trunc(d,'month')`. `aggregateRewriteByView` can produce this valid
reduced plan:
```text
UnionAll
Aggregate(month(d), SUM(s)) -> Scan MV[February]
Aggregate(month(d), SUM(v)) -> Scan t[January]
```
These groups are disjoint monthly MV buckets, but lineage here is
`DateTrunc`, so `columnGroupedByItself` returns empty and `canUnionRewrite`
discards the candidate, rescanning and reaggregating the retained data. The
earlier RANGE-overlap thread concerned the base rule's rejection of bare-column
grouping; this case passes that guard and fails in this new helper. Recognize
grouping that determines the actual MV partition transform, preserve the
per-grouping-set check, and add a partial-union regression without a redundant
bare `d` grouping key.
--
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]
