yujun777 commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4237271949
##########
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 009e1cf9f30, and you are right that `hasData` is not the version on
cloud: it answers true for a session with empty-partition pruning off before it
looks at anything. Grace reads the partition's own version now --
`getVisibleVersion() > PARTITION_INIT_VERSION` -- which on cloud is the cached
metaservice version, and normalizes to the initial version for a partition
nothing has written (`VERSION_NOT_FOUND`), so an aligned replacement is not
admitted there either. The snapshot and `hasData` are both out of this decision
now; the version is the fact itself.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRewriteUtil.java:
##########
@@ -166,25 +231,46 @@ private static Set<String>
getMtmvPartitionsByRelatedPartitions(MTMV mtmv, MTMVR
}
Set<String> pctPartitions = entry.getValue();
for (String pctPartition : pctPartitions) {
- String mvPartition = relatedToMv.get(Pair.of(tableIf,
pctPartition));
- if (mvPartition != null) {
- res.add(mvPartition);
+ Set<String> mvPartitions = relatedToMv.get(Pair.of(tableIf,
pctPartition));
+ if (mvPartitions == null) {
+ // A partition the query reads that no MV partition is
mapped from -- one the partition
+ // mapping left out, an expired base partition for
instance -- is one the MV's rows say
+ // nothing about. With the union rewrite on it is taken
out of the MV plan and read from
+ // the base table, so the MV partitions the other base
partitions map to still answer the
+ // query and this one is passed over. Without it the MV
alone would answer, so no partition
+ // is answered for at all: the MV is not a candidate for
this query and the query is
+ // answered from the base table.
+ if (unionRewrite) {
Review Comment:
Fixed in 009e1cf9f30. The pass iterated the descriptor mapping's keys, and
the query's filtering can leave a PCT table out of the mapping entirely --
every partition of it that the query reads sitting outside the window -- so
such a table was never visited and its rows were never restored. It iterates
the MV's PCT tables now, and a table the mapping does not mention at all is
read as one every query-used partition of which is uncovered.
The two-probe check stands: the earlier shape (B present in the mapping, its
`p_old` not) still returns the base table's answer, and the other suites are
unchanged.
--
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]
