yujun777 commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4268735975
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/UpdateMvByPartitionCommand.java:
##########
@@ -130,17 +142,69 @@ 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. Which of the rows that
+ // range reaches sit in an explicit partition and which in the
default one is not something a
+ // predicate on the partition columns can say -- ADD PARTITION
claims a key without moving the
+ // rows a default partition already holds under it -- so this
table is not scoped at all, and
+ // the mapping is kept whole for it for the same reason: see
+ // MTMVRelatedPartitionDescSyncLimitGenerator, which leaves
such a table out of the window.
+ // 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,
+ Optional.of(partitionColumnType(olapTable, colName))));
+ continue;
+ }
+ if (readable.isEmpty()) {
+ // No partition of this table feeds the MV partitions being
refreshed, which is "no row"
+ // rather than "every row": constructPredicates answers the
other way for an empty set,
+ // and that answer would put every row of the table into each
of them.
+ builder.put(table, Sets.newHashSet(BooleanLiteral.FALSE));
+ continue;
+ }
+ builder.put(table, constructPredicatesOfBasePartitions(items,
olapTable, colName));
Review Comment:
Fixed in 47b9c90e160. Confirmed on the code below the predicate: the roll-up
keeps that partition (`MTMVPartitionExprDateTrunc.getUpperValue` truncates the
upper bound with the last second taken off it, so a DATETIME(3) partition
ending at 2025-01-01 00:00:00.123 is the year 2024's), the read then takes the
2025-01-01 00:00:00.100 row, the sink counts it as filtered (`no partition for
this tuple`, `vtablet_finder.cpp`), and
`AbstractInsertExecutor.checkStrictModeAndFilterRatio` fails the insert since
`enable_insert_strict` is on by default.
A scoped range table is now read through the mapped partitions' bounds
**together with** the MV partitions' own key ranges, so the term is back and
the read stays inside what the batch can write; the new predicate for the case
above is
```
AND[(k1 >= '2024-01-01 00:00:00.123'),(k1 < '2025-01-01 00:00:00.123'),
(k1 >= '2024-01-01 00:00:00'),(k1 < '2025-01-01 00:00:00')]
```
Two notes on scope. A scoped **list** table keeps the whole-key pin alone: a
list partition's key is a value the MV partition's key set was written from, so
the pin is inside that range already, and adding the key set there would only
touch the NULL handling. The **union compensation** (a different predicate
builder, `PredicateAdder` / `constructPredicatesOfBasePartitions` with the
relation's slots) is deliberately left unclipped -- that branch supplies the
rows an invalid MV partition does not, so clipping it to the MV domain would
drop rows from the union rather than narrow the refresh.
Covered by
`UpdateMvByPartitionCommandTest#testAReadOfARangeBasePartitionEndsAtTheMvPartitionItRollsUpInto`
(DATETIME(3) partition ending at 2025-01-01 00:00:00.123 under a
year-granularity MV; asserts both the partition's bounds and the MV partition's
are in the read), and
`testAReadIsScopedToTheBasePartitionsTheMvPartitionIsRecordedWith` now asserts
the MV bounds too. 14 tests, all pass; both cases fail with the range left out.
A regression case for this boundary is not added yet -- I will send it as a
follow-up rather than hold this fix.
--
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]
