github-actions[bot] commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4268182713
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRewriteUtil.java:
##########
@@ -88,23 +135,49 @@ public static Collection<Partition>
getMTMVCanRewritePartitions(MTMV mtmv, Conne
if (mtmvNeedComparePartitions == null) {
try {
mtmvNeedComparePartitions =
getMtmvPartitionsByRelatedPartitions(mtmv, refreshContext,
- queryUsedPartitions);
+ queryUsedPartitions,
+
ctx.getSessionVariable().isEnableMaterializedViewUnionRewrite());
} catch (AnalysisException e) {
LOG.warn(e);
return res;
}
}
+ if (mtmvNeedComparePartitions.isEmpty()) {
+ // A base partition the query reads is mapped from no MV
partition of this one -- a partition
+ // the mapping left out, an expired one for instance -- so no
MV partition holds its rows.
+ // None of them may answer this query, whatever the state of
the partitions in grace below:
+ // the answer is the base table.
+ return Lists.newArrayList();
+ }
+ // A partition that alignment has just added is as fresh as it is
empty: a base ADD PARTITION can
+ // widen the keys an MV partition covers, which drops the
populated one and adds an empty
+ // replacement, and its creation time would let the grace period
admit it before any refresh has
+ // read it -- a query rewritten to it then reads no rows at all,
including the ones the base
+ // partitions it is to hold carry. Grace answers for a partition a
refresh has written, which the
+ // version read above says, and for no other: neither the refresh
snapshot nor `hasData` can be
+ // asked -- the snapshot is keyed by the partition's name, and
distinct keys can generate the same
+ // name, while on cloud `hasData` answers true for a session that
turned empty-partition pruning
+ // off, whatever the partition's version is.
+ if (gracePeriodMills > 0 && currentTimeMills <=
(partition.getVisibleVersionTime()
+ + gracePeriodMills) && !forceConsistent
+ && writtenPartitionsInGrace.contains(partition.getName()))
{
+ res.add(partition);
+ continue;
+ }
// if the partition which query not used, should not compare
partition version
if (!mtmvNeedComparePartitions.contains(partition.getName())) {
continue;
}
if (partitionSnapshots == null) {
+ // Every partition that reaches the sync check needs its
snapshots prepared, and what keeps a
+ // partition from reaching it is grace -- read from the same
set the check above is made of,
+ // rather than from a second reading of the grace period. A
partition inside its window that no
+ // refresh has written falls through to be compared (that is
what the version read above is for),
+ // so leaving it out here would send that comparison to
snapshots nobody prepared.
Set<String> partitionsToPreload = Sets.newHashSet();
for (Partition candidate : allPartitions) {
- boolean withinGracePeriod = gracePeriodMills > 0
- && currentTimeMills <=
candidate.getVisibleVersionTime() + gracePeriodMills
- && !forceConsistent;
- if (!withinGracePeriod &&
mtmvNeedComparePartitions.contains(candidate.getName())) {
+ if (!writtenPartitionsInGrace.contains(candidate.getName())
Review Comment:
[P2] Preload snapshots after rechecking cloud grace eligibility
The bulk version read can change visibleVersionTime:
CloudPartition.getSnapshotVisibleVersionFromMs writes the returned mtime, using
0 for the explicitly supported case where older partitions have no mtime. A
cold cloud partition whose local timestamp initially falls inside grace can
therefore return a written version, enter writtenPartitionsInGrace, then fail
the timestamp check at lines 161-163. This preload still excludes it. For a
nonempty base mapping with matching persisted snapshot names,
isMTMVPartitionSync then reaches PreparedPartitionSnapshots.get and throws
"Partition snapshot was not prepared", so an otherwise synchronized MV is
rejected and the query falls back to base scans. Compute the final grace set
after the version read and use the same set for both the continue branch and
preload; cover a bulk read that moves the timestamp outside grace.
--
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]
