github-actions[bot] commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4225709429


##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelatedPartitionDescTransferGenerator.java:
##########
@@ -46,23 +48,119 @@ public class MTMVRelatedPartitionDescTransferGenerator 
implements MTMVRelatedPar
     public void apply(MTMVPartitionInfo mvPartitionInfo, Map<String, String> 
mvProperties,
             RelatedPartitionDescResult lastResult, List<Column> 
partitionColumns,
                       Map<List<String>, Set<String>> queryUsedPartitionMap) 
throws AnalysisException {
-        Map<MTMVRelatedTableIf, Map<PartitionKeyDesc, Set<String>>> descs = 
lastResult.getDescs();
+        Map<PartitionKeyDesc, Map<MTMVRelatedTableIf, Set<String>>> res =
+                mergeOverlappingListDescs(lastResult.getDescs());
+        if (mvPartitionInfo.getPctInfos().size() > 1) {
+            checkIntersect(res.keySet(), partitionColumns);
+        }
+        lastResult.setRes(res);
+    }
+
+    /**
+     * One MV partition per set of keys that meet, whichever table wrote them 
down.
+     *
+     * <p>A partition of a list partitioned base table can hold several keys 
of the MV's partition column, so
+     * two partitions -- of one table or of two -- can describe keys that 
meet: an expired partition holding a
+     * key a retained partition also holds, for instance. An MV's own 
partitions cannot overlap, so descs
+     * whose keys meet are one partition whose keys are the union of theirs, 
and it names the partitions of
+     * every table whose keys are in it, which is what a refresh reads and 
records for those keys.
+     *
+     * <p>Merging across tables, not within each of them, is what keeps the MV 
buildable: two tables of a
+     * multi-table MV have to come out with the same descs, or one table's 
merged desc repeats a key another
+     * table's desc holds and `checkIntersect` (or the partition creation 
itself) rejects the MV.
+     *
+     * <p>Descs whose keys are disjoint stay as they are, and so does a desc 
that is the only one in its
+     * group, so an MV whose base partitions do not meet keeps its partitions 
and their names.
+     */
+    private Map<PartitionKeyDesc, Map<MTMVRelatedTableIf, Set<String>>> 
mergeOverlappingListDescs(
+            Map<MTMVRelatedTableIf, Map<PartitionKeyDesc, Set<String>>> descs) 
{
+        // A union-find over the keys: two descs whose keys meet end up in one 
group, transitively, and each
+        // key is looked up once -- walking the groups per desc would be 
quadratic in the number of partitions.
+        Map<List<PartitionValue>, List<PartitionValue>> groupOfKey = 
Maps.newHashMap();
+        for (Map<PartitionKeyDesc, Set<String>> onePctDescs : descs.values()) {
+            for (PartitionKeyDesc desc : onePctDescs.keySet()) {
+                if (!desc.hasInValues()) {
+                    continue;
+                }
+                List<PartitionValue> first = 
desc.getInValues().iterator().next();
+                groupOfKey.putIfAbsent(first, first);
+                for (List<PartitionValue> key : desc.getInValues()) {
+                    groupOfKey.putIfAbsent(key, key);
+                    union(groupOfKey, first, key);
+                }
+            }
+        }
+        Map<List<PartitionValue>, Set<List<PartitionValue>>> keysOfGroup = 
Maps.newHashMap();
+        for (List<PartitionValue> key : groupOfKey.keySet()) {
+            keysOfGroup.computeIfAbsent(find(groupOfKey, key), k -> 
Sets.newHashSet()).add(key);
+        }
+        Map<PartitionKeyDesc, List<PartitionValue>> groupOfDesc = 
Maps.newHashMap();
+        Map<List<PartitionValue>, PartitionKeyDesc> onlyDescOfGroup = 
Maps.newHashMap();
+        for (Map<PartitionKeyDesc, Set<String>> onePctDescs : descs.values()) {
+            for (PartitionKeyDesc desc : onePctDescs.keySet()) {
+                if (!desc.hasInValues()) {
+                    continue;
+                }
+                List<PartitionValue> group = find(groupOfKey, 
desc.getInValues().iterator().next());
+                groupOfDesc.put(desc, group);
+                if (onlyDescOfGroup.put(group, desc) != null) {

Review Comment:
   [P1] Keep merged LIST keys after the third descriptor. For a valid LIST(d, 
region) base with projected descriptors {1,2}, {2,3}, and {3,4}, union-find 
puts all four keys in one group, but `onlyDescOfGroup.put(group, desc)` stores 
`desc`, then `null`, then a nonnull `desc` again. The emitted MV partition has 
only the arbitrary last descriptor's keys while its mapping names all three 
base partitions, so at least one committed key has no MV destination and 
COMPLETE refresh cannot materialize it. Track group cardinality separately from 
the nullable singleton value and test a three-descriptor chain.



##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelatedPartitionDescTransferGenerator.java:
##########
@@ -46,23 +48,119 @@ public class MTMVRelatedPartitionDescTransferGenerator 
implements MTMVRelatedPar
     public void apply(MTMVPartitionInfo mvPartitionInfo, Map<String, String> 
mvProperties,
             RelatedPartitionDescResult lastResult, List<Column> 
partitionColumns,
                       Map<List<String>, Set<String>> queryUsedPartitionMap) 
throws AnalysisException {
-        Map<MTMVRelatedTableIf, Map<PartitionKeyDesc, Set<String>>> descs = 
lastResult.getDescs();
+        Map<PartitionKeyDesc, Map<MTMVRelatedTableIf, Set<String>>> res =
+                mergeOverlappingListDescs(lastResult.getDescs());
+        if (mvPartitionInfo.getPctInfos().size() > 1) {
+            checkIntersect(res.keySet(), partitionColumns);
+        }
+        lastResult.setRes(res);
+    }
+
+    /**
+     * One MV partition per set of keys that meet, whichever table wrote them 
down.
+     *
+     * <p>A partition of a list partitioned base table can hold several keys 
of the MV's partition column, so
+     * two partitions -- of one table or of two -- can describe keys that 
meet: an expired partition holding a
+     * key a retained partition also holds, for instance. An MV's own 
partitions cannot overlap, so descs
+     * whose keys meet are one partition whose keys are the union of theirs, 
and it names the partitions of
+     * every table whose keys are in it, which is what a refresh reads and 
records for those keys.
+     *
+     * <p>Merging across tables, not within each of them, is what keeps the MV 
buildable: two tables of a
+     * multi-table MV have to come out with the same descs, or one table's 
merged desc repeats a key another
+     * table's desc holds and `checkIntersect` (or the partition creation 
itself) rejects the MV.
+     *
+     * <p>Descs whose keys are disjoint stay as they are, and so does a desc 
that is the only one in its
+     * group, so an MV whose base partitions do not meet keeps its partitions 
and their names.
+     */
+    private Map<PartitionKeyDesc, Map<MTMVRelatedTableIf, Set<String>>> 
mergeOverlappingListDescs(
+            Map<MTMVRelatedTableIf, Map<PartitionKeyDesc, Set<String>>> descs) 
{
+        // A union-find over the keys: two descs whose keys meet end up in one 
group, transitively, and each
+        // key is looked up once -- walking the groups per desc would be 
quadratic in the number of partitions.
+        Map<List<PartitionValue>, List<PartitionValue>> groupOfKey = 
Maps.newHashMap();
+        for (Map<PartitionKeyDesc, Set<String>> onePctDescs : descs.values()) {
+            for (PartitionKeyDesc desc : onePctDescs.keySet()) {
+                if (!desc.hasInValues()) {
+                    continue;
+                }
+                List<PartitionValue> first = 
desc.getInValues().iterator().next();
+                groupOfKey.putIfAbsent(first, first);
+                for (List<PartitionValue> key : desc.getInValues()) {
+                    groupOfKey.putIfAbsent(key, key);
+                    union(groupOfKey, first, key);
+                }
+            }
+        }
+        Map<List<PartitionValue>, Set<List<PartitionValue>>> keysOfGroup = 
Maps.newHashMap();
+        for (List<PartitionValue> key : groupOfKey.keySet()) {
+            keysOfGroup.computeIfAbsent(find(groupOfKey, key), k -> 
Sets.newHashSet()).add(key);
+        }
+        Map<PartitionKeyDesc, List<PartitionValue>> groupOfDesc = 
Maps.newHashMap();
+        Map<List<PartitionValue>, PartitionKeyDesc> onlyDescOfGroup = 
Maps.newHashMap();
+        for (Map<PartitionKeyDesc, Set<String>> onePctDescs : descs.values()) {
+            for (PartitionKeyDesc desc : onePctDescs.keySet()) {
+                if (!desc.hasInValues()) {
+                    continue;
+                }
+                List<PartitionValue> group = find(groupOfKey, 
desc.getInValues().iterator().next());
+                groupOfDesc.put(desc, group);
+                if (onlyDescOfGroup.put(group, desc) != null) {
+                    // A second desc in this group: its keys are the group's 
from here on.
+                    onlyDescOfGroup.put(group, null);
+                }
+            }
+        }
         Map<PartitionKeyDesc, Map<MTMVRelatedTableIf, Set<String>>> res = 
Maps.newHashMap();
         for (Entry<MTMVRelatedTableIf, Map<PartitionKeyDesc, Set<String>>> 
entry : descs.entrySet()) {
-            MTMVRelatedTableIf pctTable = entry.getKey();
-            Map<PartitionKeyDesc, Set<String>> onePctDescs = entry.getValue();
-            for (Entry<PartitionKeyDesc, Set<String>> onePctEntry : 
onePctDescs.entrySet()) {
-                PartitionKeyDesc partitionKeyDesc = onePctEntry.getKey();
-                Set<String> partitionNames = onePctEntry.getValue();
-                Map<MTMVRelatedTableIf, Set<String>> partitionKeyDescMap = 
res.computeIfAbsent(partitionKeyDesc,
-                        k -> new HashMap<>());
-                partitionKeyDescMap.put(pctTable, partitionNames);
+            for (Entry<PartitionKeyDesc, Set<String>> onePctEntry : 
entry.getValue().entrySet()) {
+                PartitionKeyDesc desc = onePctEntry.getKey();
+                if (desc.hasInValues()) {
+                    List<PartitionValue> group = groupOfDesc.get(desc);
+                    PartitionKeyDesc only = onlyDescOfGroup.get(group);
+                    desc = only == null

Review Comment:
   [P2] Preserve the full merged LIST descriptor when mapping a pruned query. 
With p_single projecting {2020} and p_double projecting {2020,2038}, CREATE 
stores one MV partition {2020,2038}. A query pruned to p_single passes only 
that base partition into OnePartitionColGenerator; FOLLOW_BASE_TABLE does not 
expand the filter, so this merge emits {2020}. 
`MTMV.calculatePartitionMappings` cannot match it to the persisted {2020,2038} 
descriptor, and `getMtmvPartitionsByRelatedPartitions` rejects a usable 
rewrite. Form the full component before applying the query filter, and test a 
pruned query on the added t8 shape.



##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelatedPartitionDescTransferGenerator.java:
##########
@@ -46,23 +48,119 @@ public class MTMVRelatedPartitionDescTransferGenerator 
implements MTMVRelatedPar
     public void apply(MTMVPartitionInfo mvPartitionInfo, Map<String, String> 
mvProperties,
             RelatedPartitionDescResult lastResult, List<Column> 
partitionColumns,
                       Map<List<String>, Set<String>> queryUsedPartitionMap) 
throws AnalysisException {
-        Map<MTMVRelatedTableIf, Map<PartitionKeyDesc, Set<String>>> descs = 
lastResult.getDescs();
+        Map<PartitionKeyDesc, Map<MTMVRelatedTableIf, Set<String>>> res =
+                mergeOverlappingListDescs(lastResult.getDescs());
+        if (mvPartitionInfo.getPctInfos().size() > 1) {
+            checkIntersect(res.keySet(), partitionColumns);
+        }
+        lastResult.setRes(res);
+    }
+
+    /**
+     * One MV partition per set of keys that meet, whichever table wrote them 
down.
+     *
+     * <p>A partition of a list partitioned base table can hold several keys 
of the MV's partition column, so
+     * two partitions -- of one table or of two -- can describe keys that 
meet: an expired partition holding a
+     * key a retained partition also holds, for instance. An MV's own 
partitions cannot overlap, so descs
+     * whose keys meet are one partition whose keys are the union of theirs, 
and it names the partitions of
+     * every table whose keys are in it, which is what a refresh reads and 
records for those keys.
+     *
+     * <p>Merging across tables, not within each of them, is what keeps the MV 
buildable: two tables of a
+     * multi-table MV have to come out with the same descs, or one table's 
merged desc repeats a key another
+     * table's desc holds and `checkIntersect` (or the partition creation 
itself) rejects the MV.
+     *
+     * <p>Descs whose keys are disjoint stay as they are, and so does a desc 
that is the only one in its
+     * group, so an MV whose base partitions do not meet keeps its partitions 
and their names.
+     */
+    private Map<PartitionKeyDesc, Map<MTMVRelatedTableIf, Set<String>>> 
mergeOverlappingListDescs(
+            Map<MTMVRelatedTableIf, Map<PartitionKeyDesc, Set<String>>> descs) 
{
+        // A union-find over the keys: two descs whose keys meet end up in one 
group, transitively, and each
+        // key is looked up once -- walking the groups per desc would be 
quadratic in the number of partitions.
+        Map<List<PartitionValue>, List<PartitionValue>> groupOfKey = 
Maps.newHashMap();
+        for (Map<PartitionKeyDesc, Set<String>> onePctDescs : descs.values()) {
+            for (PartitionKeyDesc desc : onePctDescs.keySet()) {
+                if (!desc.hasInValues()) {
+                    continue;
+                }
+                List<PartitionValue> first = 
desc.getInValues().iterator().next();
+                groupOfKey.putIfAbsent(first, first);
+                for (List<PartitionValue> key : desc.getInValues()) {
+                    groupOfKey.putIfAbsent(key, key);
+                    union(groupOfKey, first, key);
+                }
+            }
+        }
+        Map<List<PartitionValue>, Set<List<PartitionValue>>> keysOfGroup = 
Maps.newHashMap();
+        for (List<PartitionValue> key : groupOfKey.keySet()) {
+            keysOfGroup.computeIfAbsent(find(groupOfKey, key), k -> 
Sets.newHashSet()).add(key);
+        }
+        Map<PartitionKeyDesc, List<PartitionValue>> groupOfDesc = 
Maps.newHashMap();
+        Map<List<PartitionValue>, PartitionKeyDesc> onlyDescOfGroup = 
Maps.newHashMap();
+        for (Map<PartitionKeyDesc, Set<String>> onePctDescs : descs.values()) {
+            for (PartitionKeyDesc desc : onePctDescs.keySet()) {
+                if (!desc.hasInValues()) {
+                    continue;
+                }
+                List<PartitionValue> group = find(groupOfKey, 
desc.getInValues().iterator().next());
+                groupOfDesc.put(desc, group);
+                if (onlyDescOfGroup.put(group, desc) != null) {
+                    // A second desc in this group: its keys are the group's 
from here on.
+                    onlyDescOfGroup.put(group, null);
+                }
+            }
+        }
         Map<PartitionKeyDesc, Map<MTMVRelatedTableIf, Set<String>>> res = 
Maps.newHashMap();
         for (Entry<MTMVRelatedTableIf, Map<PartitionKeyDesc, Set<String>>> 
entry : descs.entrySet()) {
-            MTMVRelatedTableIf pctTable = entry.getKey();
-            Map<PartitionKeyDesc, Set<String>> onePctDescs = entry.getValue();
-            for (Entry<PartitionKeyDesc, Set<String>> onePctEntry : 
onePctDescs.entrySet()) {
-                PartitionKeyDesc partitionKeyDesc = onePctEntry.getKey();
-                Set<String> partitionNames = onePctEntry.getValue();
-                Map<MTMVRelatedTableIf, Set<String>> partitionKeyDescMap = 
res.computeIfAbsent(partitionKeyDesc,
-                        k -> new HashMap<>());
-                partitionKeyDescMap.put(pctTable, partitionNames);
+            for (Entry<PartitionKeyDesc, Set<String>> onePctEntry : 
entry.getValue().entrySet()) {
+                PartitionKeyDesc desc = onePctEntry.getKey();
+                if (desc.hasInValues()) {
+                    List<PartitionValue> group = groupOfDesc.get(desc);
+                    PartitionKeyDesc only = onlyDescOfGroup.get(group);
+                    desc = only == null
+                            ? 
PartitionKeyDesc.createIn(sortedKeys(keysOfGroup.get(group))) : only;
+                }
+                res.computeIfAbsent(desc, k -> new HashMap<>())
+                        .merge(entry.getKey(), 
Sets.newHashSet(onePctEntry.getValue()), (left, right) -> {
+                            left.addAll(right);
+                            return left;
+                        });
             }
         }
-        if (mvPartitionInfo.getPctInfos().size() > 1) {
-            checkIntersect(res.keySet(), partitionColumns);
+        return res;
+    }
+
+    private void union(Map<List<PartitionValue>, List<PartitionValue>> 
groupOfKey, List<PartitionValue> left,
+            List<PartitionValue> right) {
+        List<PartitionValue> leftGroup = find(groupOfKey, left);
+        List<PartitionValue> rightGroup = find(groupOfKey, right);
+        if (leftGroup != rightGroup) {
+            groupOfKey.put(rightGroup, leftGroup);
         }
-        lastResult.setRes(res);
+    }
+
+    private List<PartitionValue> find(Map<List<PartitionValue>, 
List<PartitionValue>> groupOfKey,
+            List<PartitionValue> key) {
+        List<PartitionValue> group = 
Preconditions.checkNotNull(groupOfKey.get(key),
+                "a key is registered before it is looked up: %s", key);
+        while (group != groupOfKey.get(group)) {
+            group = groupOfKey.get(group);
+        }
+        List<PartitionValue> root = group;
+        // Path compression, so that the walk is not repeated for the rest of 
this group's keys.
+        group = groupOfKey.get(key);
+        while (group != root) {
+            List<PartitionValue> next = groupOfKey.get(group);
+            groupOfKey.put(group, root);
+            group = next;
+        }
+        return root;
+    }
+
+    /** The group's keys, in the order the base partition values sort in, so a 
partition name is stable. */
+    private List<List<PartitionValue>> sortedKeys(Set<List<PartitionValue>> 
keys) {
+        List<List<PartitionValue>> res = Lists.newArrayList(keys);
+        res.sort(Comparator.comparing(key -> key.get(0).getStringValue()));

Review Comment:
   [P1] Keep LIST key order stable before replacing an MV partition. A 
singleton LIST(k,region) partition projecting {1,2,3} uses HashSet order, while 
adding an empty overlapping base partition makes this merge sort the same key 
set. `PartitionKeyDesc.equals` treats the orders as different, so alignment 
drops the populated MV partition and adds an empty one with a new name. An 
explicit `REFRESH ... PARTITION(old_name)` resolves before alignment and then 
fails on the removed name. With `grace_period > 0`, the empty replacement's 
fresh `visibleVersionTime` lets rewrite accept it without a refresh snapshot, 
so a query over those keys can omit existing rows. Canonicalize both descriptor 
paths and test ADD followed by explicit refresh with 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]

Reply via email to