github-actions[bot] commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4145157487
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/UpdateMvByPartitionCommand.java:
##########
@@ -350,10 +555,33 @@ public Plan
visitLogicalCatalogRelation(LogicalCatalogRelation catalogRelation,
predicates.setNeedAddFilter(false);
}
if (!partitionHasDataItems.isEmpty()) {
+ // The partitions are pinned the way a refresh pins
them: the whole key of each, at
+ // the partition column's own type. A predicate on the
MV's partition column alone
+ // reads the partitions that differ in the other keys
too, and those rows are ones
+ // this branch must not add -- the MV branch of the
union already supplies them, or
+ // the compensation would count them twice -- while a
key written with a scale has to
+ // be compared at that scale or its rows are read as
none.
+ // A list partitioned table's default partition is not
pinned the way the others
+ // are: its own key is the sentinel the rows no other
partition claims were placed
+ // by, so pinning it to that key reads none of them.
It is read the way it was
+ // before the whole key was pinned -- on the MV's
partition column alone -- which
+ // can be seen to be too wide rather than one that
drops its rows.
+ boolean hasDefaultPartition =
partitionHasDataItems.stream()
+ .anyMatch(PartitionItem::isDefaultPartition);
+ Set<Expression> mvPartitionPredicates =
hasDefaultPartition
+ ? mvPartitionsToReadThrough(predicates,
relatedTableColumnInfo, partitionSlot)
+ : Sets.newHashSet();
+ Set<Expression> preds;
+ if (!mvPartitionPredicates.isEmpty()) {
+ preds = mvPartitionPredicates;
+ } else if (targetTable instanceof OlapTable &&
!hasDefaultPartition) {
+ preds =
constructPredicatesOfBasePartitions(partitionHasDataItems,
+ (OlapTable) targetTable,
relatedTableColumnInfo.getColName());
Review Comment:
[P2] Bind base partition slots in union compensation. When one RANGE or LIST
MV partition is stale and another is valid, this branch applies the full-tuple
predicate to an already bound LogicalCatalogRelation, but the helper creates
UnboundSlot values. The following whole-tree rewrite has no binding pass;
CheckAfterRewrite throws UnboundException, which the MV rule catches and
records as a failed candidate. Thus partial union rewrites over nondefault OLAP
partitions fall back to base scans. Resolve the tuple columns from
catalogRelation.getOutput() here.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/UpdateMvByPartitionCommand.java:
##########
@@ -284,6 +440,55 @@ public Plan
visitLogicalSubQueryAlias(LogicalSubQueryAlias<? extends Plan> subQu
return super.visitLogicalSubQueryAlias(subQueryAlias, predicates);
}
+ /**
+ * The ranges of the MV partitions this compensation removes, as a
predicate on the base table's
+ * partition column, or nothing when they cannot be written on one
column.
+ *
+ * <p>A default partition's rows belong to whichever MV partition
their own key falls in, so this is
+ * what they are read through: pinning them to the partition's own key
-- the sentinel those rows were
+ * placed by -- reads none of them, and reading them whole would add
the rows of the MV partitions the
+ * plan still has. Only an MV partitioned by this one column can be
written that way here.
+ */
+ private static Set<Expression>
mvPartitionsToReadThrough(PredicateAddContext predicates,
+ BaseColInfo relatedTableColumnInfo, Slot partitionSlot) {
+ Set<Expression> res = Sets.newHashSet();
+ if (predicates.getMvPartitionsToRemove().isEmpty()) {
+ return res;
+ }
+ for (Map.Entry<BaseTableInfo, Set<String>> entry
+ : predicates.getMvPartitionsToRemove().entrySet()) {
+ try {
+ TableIf table = MTMVUtil.getTable(entry.getKey());
+ if (!(table instanceof MTMV)) {
+ continue;
+ }
+ MTMV mtmv = (MTMV) table;
+ if (mtmv.getPartitionColumns().size() != 1
+ || !mtmv.getPartitionColumns().get(0).getName()
+
.equalsIgnoreCase(relatedTableColumnInfo.getColName())) {
+ continue;
+ }
+ Type columnType =
mtmv.getPartitionColumns().get(0).getType();
+ for (String partitionName : entry.getValue()) {
+ PartitionItem item =
mtmv.getPartitionItemOrAnalysisException(partitionName);
+ if (!(item instanceof ListPartitionItem)) {
+ continue;
+ }
+ List<Expression> values = ((ListPartitionItem)
item).getItems().stream()
+ .map(key -> convertPartitionKeyToLiteral(key,
0, Optional.of(columnType)))
+ .collect(Collectors.toList());
+ res.add(new InPredicate(partitionSlot, values));
+ }
Review Comment:
[P1] Preserve NULL keys in default-partition compensation. When a nullable
LIST(k) MV has a stale NULL partition and a valid k=1 partition, its default
base partition makes this helper filter the removed NULL partition as k IN
(NULL). That predicate is UNKNOWN even for NULL rows, so the union base branch
omits committed rows from p_null. Use the NULL-aware list conversion below,
which emits k IS NULL, and cover the partial-union case.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/RefreshMTMVCommand.java:
##########
@@ -198,8 +198,11 @@ protected LogicalPlan createRefreshCommand(MTMV mtmv,
StatementContext statement
statementContext.setIvmRewriteContext(Optional.of(IvmRewriteContext.fullExplain(mtmv)));
}
statementContext.setExcludedTriggerTables(mtmv.getExcludedTriggerTables());
+ // Explained through the MV partitions' own key ranges: the
read a refresh narrows to its
+ // partition mapping is decided by that refresh's context,
which a plan built here has not.
return UpdateMvByPartitionCommand.from(
- mtmv, getCompleteRefreshPartitions(mtmv),
getIncrementalTableMap(mtmv), statementContext);
+ mtmv, getCompleteRefreshPartitions(mtmv),
getIncrementalTableMap(mtmv), statementContext,
+ null);
Review Comment:
[P2] Explain the same base-partition scope as the refresh. With
partition_sync_limit, an expired base day can lie inside a retained MV year.
Actual COMPLETE refresh passes mappedBasePartitions and excludes that day, but
this null scope makes EXPLAIN REFRESH COMPLETE use the whole MV-year predicate
and show the expired partition as scanned. Build the explain predicate from the
same mapping so the plan describes the refresh it previews.
--
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]
