yujun777 commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4219924410
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/UpdateMvByPartitionCommand.java:
##########
@@ -130,17 +139,66 @@ private static List<String>
constructPartsForMv(Set<String> partitionNames) {
return Lists.newArrayList(partitionNames);
}
+ /**
+ * The predicate every base table of the MV definition is read through.
+ *
+ * <p>A table the caller scopes is read from exactly the base partitions
it named. Those are the ones
+ * the refresh is about to record as this MV partition's, and the read is
what has to match the record:
+ * reading the MV partition's own key range instead also reads base
partitions no snapshot describes,
+ * and a later silent change to one of them -- dropped, with the base
partition set back to what it
+ * was -- leaves the rows it put in this MV partition behind while the
partition is still judged
+ * synchronized, so the transparent rewrite serves them and no refresh
plans it again.
+ *
+ * <p>Every other table keeps the MV partition's own key range, which is
what the tables the caller
+ * does not scope were always read through. Scoped tables are olap ones;
the partition names are
+ * looked up on one, see the caller.
+ */
private static Map<TableIf, Set<Expression>>
constructTableWithPredicates(MTMV mv,
- Set<String> partitionNames, Map<TableIf, String> tableWithPartKey)
throws AnalysisException {
- Set<PartitionItem> items = Sets.newHashSet();
+ Set<String> partitionNames, Map<TableIf, String> tableWithPartKey,
+ Map<BaseTableInfo, Set<String>> readableBasePartitions) throws
AnalysisException {
+ Set<PartitionItem> mvItems = Sets.newHashSet();
for (String partitionName : partitionNames) {
- PartitionItem partitionItem =
mv.getPartitionItemOrAnalysisException(partitionName);
- items.add(partitionItem);
+ mvItems.add(mv.getPartitionItemOrAnalysisException(partitionName));
}
ImmutableMap.Builder<TableIf, Set<Expression>> builder = new
ImmutableMap.Builder<>();
- tableWithPartKey.forEach((table, colName) ->
- builder.put(table, constructPredicates(items, colName))
- );
+ for (Map.Entry<TableIf, String> entry : tableWithPartKey.entrySet()) {
+ TableIf table = entry.getKey();
+ String colName = entry.getValue();
+ Set<String> readable = readableBasePartitions == null ? null
+ : readableBasePartitions.get(new BaseTableInfo(table));
+ if (readable == null) {
+ builder.put(table, constructPredicates(mvItems, colName));
+ continue;
+ }
+ OlapTable olapTable = (OlapTable) table;
+ Set<PartitionItem> items = Sets.newHashSet();
+ for (String partitionName : readable) {
+
items.add(olapTable.getPartitionItemOrAnalysisException(partitionName));
+ }
+ if (items.stream().anyMatch(PartitionItem::isDefaultPartition)) {
+ // One of the partitions this MV partition is recorded with is
a list partitioned table's
+ // default partition, which takes the rows no other partition
of it claims. Those rows are
+ // the ones the MV partition's own key range names, wherever
the base table put them, and a
+ // partition of the MV takes them by that key rather than by
the partition they were placed
+ // in. So a table whose mapped partitions include one is read
the way an unscoped one is:
+ // the MV partition's key range, at the partition column's own
type. That read can be seen to
+ // be too wide -- it is the one this scope exists to narrow --
rather than one that drops
+ // rows belonging to the MV partition being refreshed. The
mapping names the default
+ // partition in every MV partition that reads the table, so
this is reached for each of them
+ // and not only for the one the sentinel key maps to.
+ builder.put(table, constructPredicates(mvItems, colName,
Review Comment:
Went with (1), in ac12fbcccb3.
The read of such a table is now the partitions the MV partition is recorded
with, pinned by their whole key, plus the rows of the MV partitions' key range
that no explicit partition the mapping does not name holds -- which is the
default partition's rows in that range, since those are the rows no explicit
partition claims. So the read is the record again, and an explicit partition
the window left out is not read at all instead of being read and left
unrecorded. The subtraction compares null-safely (`<=>`): it is the form that
gets negated, and `NOT (k = v)` is UNKNOWN rather than true for a NULL `k`,
which would have dropped the default partition's NULL-key rows along with the
ones it is meant to drop.
Measured on the shape from the report -- `LIST(d, k)` with
`p_expired=((2020-01-01, 1))`, `p_kept=((2020-01-01, 2), (2038-01-01, 2))` and
`p_default`, an MV `PARTITION BY (d)` with a 2 YEAR window: the MV partition
held `(2020-01-01, 1, 1)` before the change and does not after it, while the
kept partition's two rows and the default partition's row are there in both. It
is `list_default_scope` in `test_mtmv_base_partition_read_scope`, and it fails
with the change reverted.
What it costs, so the tradeoff is on the record: one disjunct per explicit
partition the mapping does not name inside the MV partitions' key range (a year
rollup over a daily table is of that order), and a `NOT` over them cannot drive
partition pruning -- the MV partition's key range stays the prunable part of
the predicate. (2) would have taken that cost away, but it changes the
partition model for these tables, and without the desc merge it is strictly
worse than the row it is meant to keep: an MV of this shape cannot be created
at all.
--
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]
