github-actions[bot] commented on code in PR #59972:
URL: https://github.com/apache/doris/pull/59972#discussion_r3656125993
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AsyncMaterializationContext.java:
##########
@@ -184,12 +186,19 @@ public boolean isSuccess() {
/**
* Calculate partition mappings and cache
*/
- public Map<MTMVRelatedTableIf, Map<String, Set<String>>>
calculatePartitionMappings() throws AnalysisException {
- if (partitionMultiFlatMap != null) {
+ public Map<MTMVRelatedTableIf, Map<String, Set<String>>>
calculatePartitionMappings(
Review Comment:
The cache check still performs the expensive normalization first: for an
EXPR filter, `getEffectiveQueryUsedBaseTablePartitionMap()` copies and scans
the MV/base partition metadata before `isPartitionMappingsCovered()` can return
a hit. On a miss, `mtmv.calculatePartitionMappings(rawFilter)` immediately
performs the same expansion again. Because one context can serve multiple
`StructInfo`/relationBitset rewrites, hits retain an all-partition scan and
misses pay it twice. Please compute the effective filter once and pass it into
mapping generation, while keeping a cheap raw-filter/bucket coverage index so a
hit does not normalize again; the cache test should assert expansion counts
instead of mocking this boundary away.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRefreshContext.java:
##########
@@ -58,9 +59,10 @@ public Map<BaseTableInfo, MTMVSnapshotIf>
getBaseTableSnapshotCache() {
return baseTableSnapshotCache;
}
- public static MTMVRefreshContext buildContext(MTMV mtmv) throws
AnalysisException {
+ public static MTMVRefreshContext buildContext(MTMV mtmv, Map<List<String>,
Set<String>> queryUsedPartitions)
+ throws AnalysisException {
MTMVRefreshContext context = new MTMVRefreshContext(mtmv);
- context.partitionMappings = mtmv.calculatePartitionMappings();
+ context.partitionMappings =
mtmv.calculatePartitionMappings(queryUsedPartitions);
Review Comment:
The query filter stops at mapping generation: immediately afterward
`getBaseVersions(mtmv)` copies every OLAP PCT partition and passes the complete
list to `Partition.getVisibleVersions()`. In cloud mode that inspects the cache
state of every partition and requests every expired entry from the meta-service
(or all entries when the cache is disabled). `isSyncWithPartitions()` and
refresh snapshot generation later read only names present in the completed
mappings, so a one-bucket query over a `B`-partition PCT still performs `O(B)`
local/cache work and can issue a `B`-entry remote request. Please derive each
PCT table's union of mapped names and snapshot only those. Missing/null and
empty-filter maintenance contexts remain safe because their completed mappings
still contain every PCT name those contexts can later request; preserve the
separate non-PCT table-version snapshots. Add a test asserting the number of
partitions passed to visible-version collection for a small selected buck
et.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVPartitionExpander.java:
##########
@@ -0,0 +1,122 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.mtmv;
+
+import org.apache.doris.catalog.PartitionItem;
+import org.apache.doris.catalog.PartitionKey;
+import org.apache.doris.catalog.PartitionType;
+import org.apache.doris.catalog.RangePartitionItem;
+import org.apache.doris.common.AnalysisException;
+import org.apache.doris.datasource.mvcc.MvccSnapshot;
+import org.apache.doris.datasource.mvcc.MvccUtil;
+
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.collect.Sets;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Optional;
+import java.util.Set;
+
+/**
+ * Utility to expand query-used partition filters to MV partition granularity
+ * using Range.encloses(), avoiding expensive dateTrunc / strToDate /
dateIncrement
+ * per-partition operations in the rollup pipeline.
+ * Separated from MTMV to keep a lightweight dependency tree for testability —
+ * loading this class does not trigger MTMV → OlapTable → CloudReplica class
loading.
+ */
+public class MTMVPartitionExpander {
+
+ /**
+ * Expand queryUsedPartitions to MV partition granularity for RANGE base
tables.
+ * For example, if MV is monthly partitioned (date_trunc(month)) and base
table is daily:
+ * - Query uses p_20250115 (Jan 15)
+ * - Find MV partition p_202501 that encloses [20250115, 20250116)
+ * - Expand to ALL daily partitions within p_202501's range [20250101,
20250201)
+ * - Result: {p_20250101, p_20250102, ..., p_20250131}
+ */
+ public static Map<List<String>, Set<String>>
expandToMvPartitionGranularity(
+ Map<List<String>, Set<String>> queryUsedBaseTablePartitionMap,
+ Map<String, PartitionItem> mvPartitionItems,
+ Set<MTMVRelatedTableIf> pctTables) throws AnalysisException {
+ List<Range<PartitionKey>> mvRanges = new
ArrayList<>(mvPartitionItems.size());
+ for (PartitionItem item : mvPartitionItems.values()) {
+ mvRanges.add(((RangePartitionItem) item).getItems());
+ }
+
+ Map<List<String>, Set<String>> expanded = Maps.newHashMap();
+ for (MTMVRelatedTableIf pctTable : pctTables) {
+ List<String> qualifiers = pctTable.getFullQualifiers();
+ Set<String> queryUsedPartitions =
queryUsedBaseTablePartitionMap.get(qualifiers);
+ if (queryUsedPartitions == null) {
+ continue;
+ }
+
+ Optional<MvccSnapshot> snapshot =
MvccUtil.getSnapshotFromContext(pctTable);
+ if (pctTable.getPartitionType(snapshot) != PartitionType.RANGE) {
+ expanded.put(qualifiers, queryUsedPartitions);
+ continue;
+ }
+
+ Map<String, PartitionItem> basePartitionItems =
pctTable.getAndCopyPartitionItems(snapshot);
+
+ List<Range<PartitionKey>> relevantMvRanges = new ArrayList<>();
+ for (String queriedBasePartition : queryUsedPartitions) {
+ PartitionItem baseItem =
basePartitionItems.get(queriedBasePartition);
+ if (baseItem == null) {
+ continue;
+ }
+ Range<PartitionKey> baseRange = ((RangePartitionItem)
baseItem).getItems();
Review Comment:
This expansion is quadratic for the broad internal-table scans this PR is
intended to speed up. `QueryPartitionCollector` records every selected OLAP
partition name, so the first nested loop does `Q * M` enclosure checks; after
collecting `R` buckets, the second loop does another `B * R`. A query selecting
most of an `N`-partition table can therefore do `O(N^2)` range comparisons
before the existing generator pipeline. Please index/sort the non-overlapping
ranges (binary lookup plus a sorted sweep is sufficient), and bypass expansion
when the explicit selection covers the whole table. A many-partition test
should cover both sparse and broad filters.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVPartitionExpander.java:
##########
@@ -0,0 +1,122 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.mtmv;
+
+import org.apache.doris.catalog.PartitionItem;
+import org.apache.doris.catalog.PartitionKey;
+import org.apache.doris.catalog.PartitionType;
+import org.apache.doris.catalog.RangePartitionItem;
+import org.apache.doris.common.AnalysisException;
+import org.apache.doris.datasource.mvcc.MvccSnapshot;
+import org.apache.doris.datasource.mvcc.MvccUtil;
+
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.collect.Sets;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Optional;
+import java.util.Set;
+
+/**
+ * Utility to expand query-used partition filters to MV partition granularity
+ * using Range.encloses(), avoiding expensive dateTrunc / strToDate /
dateIncrement
+ * per-partition operations in the rollup pipeline.
+ * Separated from MTMV to keep a lightweight dependency tree for testability —
+ * loading this class does not trigger MTMV → OlapTable → CloudReplica class
loading.
+ */
+public class MTMVPartitionExpander {
+
+ /**
+ * Expand queryUsedPartitions to MV partition granularity for RANGE base
tables.
+ * For example, if MV is monthly partitioned (date_trunc(month)) and base
table is daily:
+ * - Query uses p_20250115 (Jan 15)
+ * - Find MV partition p_202501 that encloses [20250115, 20250116)
+ * - Expand to ALL daily partitions within p_202501's range [20250101,
20250201)
+ * - Result: {p_20250101, p_20250102, ..., p_20250131}
+ */
+ public static Map<List<String>, Set<String>>
expandToMvPartitionGranularity(
+ Map<List<String>, Set<String>> queryUsedBaseTablePartitionMap,
+ Map<String, PartitionItem> mvPartitionItems,
+ Set<MTMVRelatedTableIf> pctTables) throws AnalysisException {
+ List<Range<PartitionKey>> mvRanges = new
ArrayList<>(mvPartitionItems.size());
+ for (PartitionItem item : mvPartitionItems.values()) {
+ mvRanges.add(((RangePartitionItem) item).getItems());
+ }
+
+ Map<List<String>, Set<String>> expanded = Maps.newHashMap();
+ for (MTMVRelatedTableIf pctTable : pctTables) {
Review Comment:
Expanding each PCT table independently leaves multi-PCT mappings incomplete.
If both tables have partitions in MV buckets X and Y but the query selects A/X
and B/Y, this loop expands A only to X and B only to Y; Transfer emits X
without B and Y without A. `isMTMVPartitionSync()` still checks every PCT table
and treats the missing mapping as empty, which differs from the full-refresh
snapshot, so both otherwise current MV buckets are rejected. Build the union of
MV buckets selected by any PCT filter first (a present null/all entry selects
all), include every PCT table's corresponding partitions for each selected
bucket, and key cache coverage by that same bucket union. Please add an
end-to-end multi-PCT test with different per-table filters.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelatedPartitionDescSyncLimitGenerator.java:
##########
@@ -45,7 +46,8 @@ public class MTMVRelatedPartitionDescSyncLimitGenerator
implements MTMVRelatedPa
@Override
public void apply(MTMVPartitionInfo mvPartitionInfo, Map<String, String>
mvProperties,
- RelatedPartitionDescResult lastResult, List<Column>
partitionColumns) throws AnalysisException {
+ RelatedPartitionDescResult lastResult, List<Column>
partitionColumns,
Review Comment:
With `partition_sync_limit` enabled, this new filter is not used until the
following `OnePartitionCol` stage. `SyncLimit` first walks every partition of
every PCT table and calls `isGreaterThanSpecifiedTime()`, which performs
endpoint/date conversion (and can evaluate `str_to_date` for string-backed
dates), only for the next generator to discard unrelated names. Intersect each
table's items with its non-null selected-name set before the cutoff conversion;
preserve missing/null as the established full-table path and explicit empty as
none. A combined sync-limit plus sparse-query test should verify how many items
are evaluated.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java:
##########
@@ -530,15 +553,25 @@ public Map<String, PartitionKeyDesc>
generateMvPartitionDescs() {
* @return mvPartitionName ==> pctTable ==> pctPartitionName
* @throws AnalysisException
*/
- public Map<String, Map<MTMVRelatedTableIf, Set<String>>>
calculatePartitionMappings() throws AnalysisException {
+ public Map<String, Map<MTMVRelatedTableIf, Set<String>>>
calculatePartitionMappings(
+ Map<List<String>, Set<String>> queryUsedBaseTablePartitionMap)
throws AnalysisException {
if (mvPartitionInfo.getPartitionType() ==
MTMVPartitionType.SELF_MANAGE) {
return Maps.newHashMap();
}
long start = System.currentTimeMillis();
+ // For EXPR-type partitions with RANGE base tables, expand the
query-used partition
+ // filter to MV partition granularity. This ensures complete partition
mappings per
+ // MV partition (needed for isSyncWithPartitions correctness) while
skipping
+ // irrelevant MV partitions entirely (the performance optimization).
+ // For nested MVs where pctTable is not in the filter, the expanded
map is empty,
+ // so the pipeline runs without filtering (full computation) — correct
behavior.
+ Map<String, PartitionItem> mvPartitionItems =
getAndCopyPartitionItems();
+ Map<List<String>, Set<String>> effectiveFilter
+ =
getEffectiveQueryUsedBaseTablePartitionMap(queryUsedBaseTablePartitionMap,
mvPartitionItems);
Map<String, Map<MTMVRelatedTableIf, Set<String>>> res =
Maps.newHashMap();
Map<PartitionKeyDesc, Map<MTMVRelatedTableIf, Set<String>>>
pctPartitionDescs = MTMVPartitionUtil
- .generateRelatedPartitionDescs(mvPartitionInfo, mvProperties,
getPartitionColumns());
- Map<String, PartitionItem> mvPartitionItems =
getAndCopyPartitionItems();
Review Comment:
Filtering `pctPartitionDescs` does not reduce the returned mapping
cardinality: this loop still inserts one entry for every physical MV partition
and allocates an empty map for every non-match, after which
`AsyncMaterializationContext` walks all of them. A one-bucket query over an
`M`-partition MV therefore retains `O(M)` allocation and iteration, contrary to
the comment that irrelevant partitions are skipped entirely. In filtered
rewrite mode, emit only physical partitions present in `pctPartitionDescs`;
preserve the full result only for unfiltered refresh/metadata callers, and add
an end-to-end cardinality assertion.
--
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]
