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/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.
##########
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/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.
--
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]
