yujun777 commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4366067620
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java:
##########
@@ -505,45 +504,73 @@ 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 by the columns the view is
partitioned on, each of them by
+ * itself, keeps the two apart -- the view's partitions are disjoint in
that tuple, by value for a list
+ * partitioned view and by range for a range partitioned one. Grouping by
an expression over a partition
+ * column 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 a partition column out -- the {@code ()} total of {@code
ROLLUP}/{@code CUBE}/{@code GROUPING
+ * SETS} puts every partition value in one group, which both branches
answer, and two partitions of a view
+ * partitioned on two columns can share one value of one of them.
*/
@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;
- }
- String columnName = ((SlotReference)
expr).getOriginalColumn().map(Column::getName).orElse(null);
- BaseTableInfo baseTableInfo = ((SlotReference)
expr).getOriginalTable().map(BaseTableInfo::new)
- .orElse(null);
- return pctInfos.stream().anyMatch(colInfo ->
colInfo.getTableInfo().equals(baseTableInfo)
- && colInfo.getColName().equals(columnName));
- }
- ).isEmpty();
- if (canUnionRewrite) {
- break;
+ List<BaseColInfo> pctInfos =
context.getMtmv().getMvPartitionInfo().getPctInfos();
+ Optional<LogicalRepeat<?>> logicalRepeatOptional =
queryAggregate.getSourceRepeat();
+ if (!logicalRepeatOptional.isPresent()) {
+ return groupingCoversPartitionColumns(groupByExpressions,
queryPlan, pctInfos);
+ }
+ // 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)) {
+ return false;
}
}
- return canUnionRewrite;
+ return true;
+ }
+
+ /**
+ * Whether these grouping expressions hold every column the MV is
partitioned on, each of them by itself:
+ * an expression over a partition column puts several of its values in one
group, and one column of a
+ * multi-column partitioning leaves the other to vary within a group.
+ */
+ private static boolean groupingCoversPartitionColumns(List<Expression>
groupingExpressions, Plan queryPlan,
+ List<BaseColInfo> pctInfos) {
+ List<? extends Expression> groupingShuttledExpressions =
+
ExpressionUtils.shuttleExpressionWithLineage(groupingExpressions, queryPlan);
+ return pctInfos.stream().allMatch(colInfo ->
groupingShuttledExpressions.stream()
Review Comment:
Fixed in `927f0129f88`.
`canUnionRewrite` now closes the grouping over the equalities the query
guarantees, instead of asking every
recorded column to appear in it by itself:
```java
// 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);
```
`columnsTheQueryHoldsEqual` walks the query plan's filters and inner joins
and takes the equalities between
two bare table columns out of their conjuncts, through the machinery that is
already there for this
(`Predicates.splitPredicates(ExpressionUtils.and(guaranteed)).getEqualPredicateMap()`).
`StructInfo.getEquivalenceClass()` could not be reused: it is derived by
`predicatesDerive`, which collects
only what sits on a `LogicalFilter`, so a join's on-predicate is never in it
-- and the on-predicate is where
this equality lives. `groupingCoversPartitionColumns` then walks the closure
from the grouped partition
columns and requires it to reach every entry of `pctInfos`, so `GROUP BY
a.d` covers `{A.d, B.d}` through the
join's `a.d = b.d`.
What it does not accept, and why: an outer join's on-predicate, since a
null-supplied row does not satisfy it;
an equality under a disjunction, which `splitPredicates` leaves in the
residual set; and an equality over
anything but a bare column of a table, so `cast(a.d as datetime) = b.d` ties
nothing. A partition column is
never nullable, so the closure cannot meet in a NULL group either. The
per-grouping-set check is kept: each
set is closed on its own, so a `ROLLUP`/`CUBE` total that leaves the column
out still declines.
The case is the new `pct_join_one_side` fixture of
`mtmv_p0/test_mtmv_base_partition_read_scope`:
`pct_join_a` and `pct_join_b` both RANGE partitioned into aligned daily
`p_old`, `p_kept` and `p_third`, with
`PARTITION BY (a_d) AS SELECT a.d AS a_d, SUM(a.v) AS s FROM pct_join_a a
JOIN pct_join_b b ON a.d = b.d
GROUP BY a.d` over a 7 DAY window, and 1000 more written into `p_old` of
`pct_join_a` after the refresh. The
view records both `A.d` and `B.d`; the query groups by `a.d` alone. Its
`explain` asserts `UNION` and the view,
and the golden is one row per day -- the day reported as a `DATEDIFF` offset
so the rows hold on any date the
suite runs on.
What was measured, on a local cluster with that fixture: with the previous
version the candidate is
declined -- the fail info reads `View struct info is invalid, need
compensate union all, but can not` and the
plan scans `p_old,p_kept` of both tables with no view in it; with this
change the plan is
`MV[kept] ∪ (pct_join_a[p_old] ⋈ pct_join_b[p_old])` and the query's rows
are what the same query gives with
the rewrite off.
Validation: `mtmv_p0/test_mtmv_base_partition_read_scope` runs the new
fixture and passes (the golden above is
what it generated), and the mv/mtmv FE unit test classes -- 69 classes, 1075
tests -- pass. The rest of the
union-rewrite suites are the ones this branch already ran green, and CI
covers them again here.
--
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]
