github-actions[bot] commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4225184320
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelatedPartitionDescTransferGenerator.java:
##########
@@ -46,7 +47,8 @@ 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<MTMVRelatedTableIf, Map<PartitionKeyDesc, Set<String>>> descs =
+ mergeOverlappingListDescs(lastResult.getDescs());
Map<PartitionKeyDesc, Map<MTMVRelatedTableIf, Set<String>>> res =
Maps.newHashMap();
Review Comment:
[P1] Merge overlapping LIST keys across PCT tables before checking
intersection. A two-PCT MV can have A's LIST(d, region) partitions projected to
{D1,D2} and {D2,D3} plus p_default, while B has {D2,D3}. With a sync window
that excludes the older D1/D2 partition and retains D3, the old pipeline kept
only A's {D2,D3}, so the MV was valid. Skipping A's window and merging it per
table now yields {D1,D2,D3} beside B's {D2,D3}; checkIntersectForList rejects
the repeated D2/D3 during CREATE or the next refresh alignment of an existing
MV. Merge the combined key space while retaining both tables' partition names,
and cover a two-PCT default/window fixture.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelatedPartitionDescTransferGenerator.java:
##########
@@ -65,6 +70,68 @@ public void apply(MTMVPartitionInfo mvPartitionInfo,
Map<String, String> mvPrope
lastResult.setRes(res);
}
+ /**
+ * One MV partition per set of keys, not one per way of writing a set
down. A partition of a list
+ * partitioned base table can hold several keys of the MV's partition
column, so two of them can describe
+ * keys that meet: an expired partition holding one key of a retained
partition's key list, for instance.
+ * An MV's own partitions cannot overlap, so descs whose keys meet are one
partition whose keys are the
+ * union of theirs. Without this an MV over such a table cannot be built
at all -- its partition items
+ * would repeat a key -- which is the shape a default-partition table is
left unwindowed into, and the
+ * one it is recorded with changes with it: the merged partition names
every base partition of the keys
+ * it covers, which is what a refresh reads for it.
+ *
+ * <p>Descs whose keys are disjoint, one desc per table, and every desc
that is not a list of keys are
+ * left exactly as they were, so an MV whose base partitions do not meet
keeps its partitions and their
+ * names.
+ */
+ private Map<MTMVRelatedTableIf, Map<PartitionKeyDesc, Set<String>>>
mergeOverlappingListDescs(
+ Map<MTMVRelatedTableIf, Map<PartitionKeyDesc, Set<String>>> descs)
{
+ Map<MTMVRelatedTableIf, Map<PartitionKeyDesc, Set<String>>> res =
Maps.newHashMap();
+ for (Entry<MTMVRelatedTableIf, Map<PartitionKeyDesc, Set<String>>>
entry : descs.entrySet()) {
+ res.put(entry.getKey(),
mergeOverlappingListDescsOfOneTable(entry.getValue()));
+ }
+ return res;
+ }
+
+ private Map<PartitionKeyDesc, Set<String>>
mergeOverlappingListDescsOfOneTable(
+ Map<PartitionKeyDesc, Set<String>> descs) {
+ Map<PartitionKeyDesc, Set<String>> res = Maps.newHashMap();
+ List<Set<List<PartitionValue>>> mergedKeys = Lists.newArrayList();
+ List<Set<String>> mergedNames = Lists.newArrayList();
+ List<PartitionKeyDesc> mergedDescs = Lists.newArrayList();
+ for (Entry<PartitionKeyDesc, Set<String>> entry : descs.entrySet()) {
+ if (!entry.getKey().hasInValues()) {
+ res.put(entry.getKey(), entry.getValue());
+ continue;
+ }
+ Set<List<PartitionValue>> keys =
Sets.newHashSet(entry.getKey().getInValues());
+ Set<String> names = Sets.newHashSet(entry.getValue());
+ // The desc this group came from, kept while it is the only one,
since a desc that was not
+ // merged is left as it is rather than written out again in
another key order.
+ PartitionKeyDesc mergedDesc = entry.getKey();
+ for (int i = mergedKeys.size() - 1; i >= 0; i--) {
+ if (Collections.disjoint(mergedKeys.get(i), keys)) {
Review Comment:
[P2] Avoid scanning every prior LIST group for each descriptor. For N
ordinary disjoint single-key partitions, this loop makes N(N-1)/2 disjoint
checks; the default 10,000-partition limit allows roughly 50 million checks for
one mapping. Rewrite candidate selection builds this mapping even when every MV
partition is within its grace period, so queries can pay the cost repeatedly.
Index each projected key to its group and union only groups that actually share
a key.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java:
##########
@@ -1619,6 +1621,65 @@ public Map<String, Map<MTMVRelatedTableIf, Set<String>>>
calculatePartitionMappi
return res;
}
+ /**
+ * The list partition each base table of this MV has that takes the rows
no other partition of it claims,
+ * by table, or none for a table that has no such partition.
+ *
+ * <p>Read once per mapping rather than per MV partition: the mapping
describes every MV partition and the
+ * answer is the table's, not the partition's. The table's partitions are
read under its read lock, so
+ * that a concurrent ADD or DROP PARTITION cannot be seen half applied --
its name list and the items the
+ * walk resolves against it have to come from one state of the table --
and so that this walk is not one
+ * more reader of a tree another thread is modifying.
+ */
+ private Map<MTMVRelatedTableIf, String> defaultListPartitionsOf() throws
AnalysisException {
+ Map<MTMVRelatedTableIf, String> res = Maps.newHashMap();
+ for (MTMVRelatedTableIf pctTable : mvPartitionInfo.getPctTables()) {
+ if (!(pctTable instanceof OlapTable)) {
+ continue;
+ }
+ OlapTable olapTable = (OlapTable) pctTable;
+ if (!(olapTable.getPartitionInfo() instanceof ListPartitionInfo)) {
+ continue;
+ }
+ olapTable.readLock();
+ try {
+ for (String partitionName : olapTable.getPartitionNames()) {
+ if
(olapTable.getPartitionItemOrAnalysisException(partitionName).isDefaultPartition())
{
+ res.put(pctTable, partitionName);
+ break;
+ }
+ }
+ } finally {
+ olapTable.readUnlock();
+ }
+ }
+ return res;
+ }
+
+ /**
+ * One MV partition's mapping, with every base table's default list
partition named in it.
+ *
+ * <p>Such a partition holds rows for every key its table can be read by,
so it belongs to every MV
+ * partition that reads the table -- not only to the one its own key, the
sentinel those rows were placed
+ * by, maps to. Naming it everywhere is what the read and the record have
to agree on: the refresh reads
+ * the rows of it that belong to the MV partition being refreshed, and the
partition is recorded among the
+ * ones that partition is read through, so an insert into it leaves that
MV partition out of sync instead
+ * of changing nothing the MV compares.
+ */
+ private Map<MTMVRelatedTableIf, Set<String>> withDefaultListPartitions(
+ Map<MTMVRelatedTableIf, Set<String>> mapping,
Map<MTMVRelatedTableIf, String> defaultListPartitions) {
+ if (defaultListPartitions.isEmpty()) {
+ return mapping;
+ }
+ Map<MTMVRelatedTableIf, Set<String>> res = Maps.newHashMap(mapping);
+ for (Entry<MTMVRelatedTableIf, String> entry :
defaultListPartitions.entrySet()) {
+ Set<String> partitions =
Sets.newHashSet(res.getOrDefault(entry.getKey(), Sets.newHashSet()));
+ partitions.add(entry.getValue());
+ res.put(entry.getKey(), partitions);
Review Comment:
[P1] Give default-only keys an MV destination before treating this fanout as
complete. For a LIST(k) base with p1=(1) and p_default, a committed k=2 row
lands in p_default. Descriptor generation gives the MV only key-1 and
synthetic-minimum partitions; adding p_default to both mappings records a
dependency, but COMPLETE refresh filters each scan by its MV key, so neither
reads k=2. The partitions can still appear synchronized; an unrestricted query
using both base partitions can rewrite to this MV and omit the committed k=2
group. The added regression covers a default row sharing an explicit MV key;
cover this distinct key and either represent it in the MV or refuse the
rewrite. This representation gap predates the fanout change but remains in this
path.
--
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]
