yujun777 commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4237102116
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRewriteUtil.java:
##########
@@ -94,6 +111,18 @@ public static Collection<Partition>
getMTMVCanRewritePartitions(MTMV mtmv, Conne
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();
+ }
+ if (gracePeriodMills > 0 && currentTimeMills <=
(partition.getVisibleVersionTime()
+ + gracePeriodMills) && !forceConsistent) {
Review Comment:
Fixed in 583dbdac6c3, and you are right that the check was on the wrong
thing: the snapshot is keyed by the partition's *name*, and a name is generated
from the keys, so distinct keys that generate the same one -- `a-b` and `ab`
both giving `p_ab` -- leave the replaced partition's entry behind for the
replacement to inherit.
What grace is about is whether a refresh has written the partition, which
the partition's own version says and no name is involved in: a written
partition's version, even one a refresh left empty, is not the one it started
at, while an aligned replacement keeps that one. Grace reads that now and does
not consult the snapshot at all. That covers the rename case the snapshot check
was added for as well -- a renamed MV keeps its partitions' versions -- and it
no longer depends on `hasData` meaning rows: what it reads is the version.
The suites show it: in `union_rewrite_grace_big`, with a base partition
added after the MV's refresh, `query_7_0_after_no_sql_cache`,
`query_17_0_after_no_sql_cache` and `query_19_0_after_no_sql_cache` now match
their `before` (base table) blocks row for row -- they were a row short before,
and their goldens are regenerated accordingly. `MTMVRewriteUtilTest` pins the
case you describe: a partition in grace whose name the snapshot records but
which nothing wrote is not answered for.
--
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]
