yujun777 commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4280794530
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/PartitionCompensator.java:
##########
@@ -144,15 +148,85 @@ public static Pair<Map<BaseTableInfo, Set<String>>,
Map<BaseColInfo, Set<String>
if (allCompensateIsNull) {
return null;
}
+ // A base partition the query reads that no MV partition of this plan
is named from -- one the
+ // partition_sync_limit window left out, say -- is one the MV does not
answer for: a partition of the MV
+ // reads a table through the partitions it does name, and the mapping
has no entry for this one. Its
+ // rows have to come from the base table, and two things follow from
that, decided apart.
+ //
+ // The base branch of the union reads those partitions. A table the
mapping mentions not at all is
+ // uncovered in every partition the query uses, so it is read for the
query's own partitions there
+ // rather than skipped -- without this the plan would answer for that
table out of the MV alone.
+ //
+ // And the MV partitions this plan uses have to be taken out when they
hold rows of such a partition: a
+ // partition the window has since left out can have been materialized
in one of them by an earlier
+ // refresh, and the union would then count its rows twice. The refresh
snapshot names exactly the
+ // partitions a refresh read into each MV partition, so that is what
this is asked of. (It is not the
+ // keys: a list partition's default one holds the keys no other
partition of the table claims, so a
+ // partition can be read into an MV partition whose keys do not cover
it.)
+ Map<MTMVRelatedTableIf, Set<String>> uncoveredByTable =
Maps.newLinkedHashMap();
+ boolean uncoveredRecorded = false;
+ for (MTMVRelatedTableIf pctTable :
mtmv.getMvPartitionInfo().getPctTables()) {
+ Set<String> queryUsed =
queryUsedBaseTablePartitionMap.get(pctTable.getFullQualifiers());
+ if (CollectionUtils.isEmpty(queryUsed)) {
+ continue;
+ }
+ // The mapping of a table the query's filtering left out entirely
is empty, so a table it does not
+ // mention at all is a table every query-used partition of which
is uncovered.
+ Map<String, Set<String>> tableMapping =
mtmvRelatedTableIfMapMap.getOrDefault(pctTable, Maps.newHashMap());
+ Set<String> named = Sets.newHashSet();
+ for (Set<String> names : tableMapping.values()) {
+ named.addAll(names);
+ }
+ Set<String> uncovered = Sets.difference(queryUsed, named);
+ if (uncovered.isEmpty()) {
+ continue;
+ }
+ uncoveredByTable.put(pctTable, uncovered);
+ Set<String> recorded = Sets.newHashSet();
+ MTMVRefreshSnapshot refreshSnapshot = mtmv.getRefreshSnapshot();
+ if (refreshSnapshot != null) {
+ for (String mvPartitionName :
rewrittenPlanUsePartitionNameSet) {
+
recorded.addAll(refreshSnapshot.getPctSnapshots(mvPartitionName, new
BaseTableInfo(pctTable)));
+ }
+ }
+ if (!Sets.intersection(uncovered, recorded).isEmpty()) {
+ uncoveredRecorded = true;
+ }
+ }
+ if (!uncoveredByTable.isEmpty()) {
+ for (Entry<MTMVRelatedTableIf, Set<String>> uncoveredEntry :
uncoveredByTable.entrySet()) {
+ BaseColInfo pctInfo = pctInfoMap.get(new
BaseTableInfo(uncoveredEntry.getKey()));
+ if (pctInfo == null) {
+ continue;
+ }
+ // With the MV partitions taken out the MV answers none of
what the query reads, so the whole of
+ // the query's partitions comes from the base; otherwise the
MV answers the partitions the
+ // mapping names and the base branch reads the ones it does
not.
+ Set<String> toRead = uncoveredRecorded
+ ? queryUsedBaseTablePartitionMap.getOrDefault(
+ uncoveredEntry.getKey().getFullQualifiers(),
Sets.newHashSet())
+ : uncoveredEntry.getValue();
Review Comment:
Fixed in b08a1f34026. You are right about where the count read, and the
count is what the decision turns on: `AbstractMaterializedViewRule` passes
`rewrittenPlan`, which at that point is the query rewritten by the MV (the
partition-pruning rewrite just above it), so the query's scans of the PCT
tables are already replaced by the MV scan and the count was 0 for essentially
every shape -- for a self-join only the `narrowedTables > 1` term could ever
fire, and it is 1 there. `PartitionCompensator#pctInputsInQuery` now counts the
relations of `queryStructInfo` itself, and `calcInvalidPartitions` takes the
number as a parameter.
`PartitionCompensatorTest#testPctInputsInQueryCountsEveryAlias` pins the count
(one table read twice is two inputs, a table the MV is not partitioned on is
none) and `...ReadsEveryInputOfASelfJoinTable` pins the decision for the
one-narrowed-input shape: the plan's MV partitions are taken out and every
query partition is read from the base.
One measurement against the finding itself, which is why the result case is
not in this commit: I could not get the end-to-end shape to reach the wrong
answer on a local cluster. With a `LIST(d,k)` table whose `p_old =
{(2020-01-01,2)}` the 2 YEAR window drops, one MV partitioned by `d` over `t a
JOIN t b ON a.d = b.d`, and the query on `a.d = '2020-01-01'` (two matching
rows on each side, so four pairs are expected), `explain memo plan` reports the
candidate as `MaterializedViewRewriteSuccessButNotChose` and the answer comes
from the base plan -- and it does so with the count forced to 0 as a control,
i.e. with the old reading, so the narrowed candidate is not chosen in that
shape either way and a case in it would pass under both. The decision is pinned
at the unit level instead. If you have a shape where the union rewrite is
chosen for a self-join, I will add the result case to it.
--
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]
