github-actions[bot] commented on code in PR #66795:
URL: https://github.com/apache/doris/pull/66795#discussion_r3804584886
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/RefreshMTMVInfo.java:
##########
@@ -97,18 +98,64 @@ private void checkPartitionExist(MTMV mtmv) throws
org.apache.doris.common.Analy
"The partition method of this asynchronous
materialized view "
+ "does not support refreshing by partition");
}
- List<AllPartitionDesc> partitionDescs =
MTMVPartitionUtil.getPartitionDescsByRelatedTable(
- mtmv.getTableProperty().getProperties(),
mtmv.getMvPartitionInfo(), mtmv.getMvProperties(),
- mtmv.getPartitionColumns());
- Set<String> shouldExistPartitionNames =
Sets.newHashSetWithExpectedSize(partitionDescs.size());
- partitionDescs.stream().forEach(desc -> {
- shouldExistPartitionNames.add(((SinglePartitionDesc)
desc).getPartitionName());
- });
+ // First validate against the real physical partition names
already stored in the MTMV metadata.
+ // SHOW PARTITIONS returns these names, and MVs created before
partition name generation was made
+ // deterministic may carry a historical time suffix, so
regenerating names here could produce a
+ // different string than the stored one and wrongly reject a valid
refresh request.
+ Set<String> existPartitionNames = mtmv.getPartitionNames();
+ // Secondly validate against the partition names that would be
generated (and aligned) from the
+ // related base table partition descs, so that refreshing a
not-yet-created partition is allowed.
+ Set<PartitionKeyDesc> relatedPartitionDescs =
MTMVPartitionUtil.generateRelatedPartitionDescs(
+ mtmv.getMvPartitionInfo(), mtmv.getMvProperties(),
mtmv.getPartitionColumns(),
+ Maps.newHashMap()).keySet();
+ Set<String> shouldExistPartitionNames =
relatedPartitionDescs.stream()
+ .map(MTMVPartitionUtil::generatePartitionName)
+ .collect(Collectors.toSet());
+ // Map every stored physical partition desc back to its physical
name. A regenerated (alias)
+ // name whose descriptor is already physically present under a
legacy time-suffixed name must be
+ // remapped to that physical name, otherwise alignMvPartition sees
the descriptor as already
+ // represented (and adds nothing) while
calculateNeedRefreshPartitions drops the nonphysical
+ // alias, and the manual refresh completes as NOT_REFRESH without
refreshing anything.
+ Map<PartitionKeyDesc, String> descToPhysicalName =
Maps.newHashMap();
+ for (String partitionName : existPartitionNames) {
+ descToPhysicalName.putIfAbsent(
+
mtmv.getPartitionItemOrAnalysisException(partitionName).toPartitionKeyDesc(),
+ partitionName);
+ }
+ List<String> resolvedPartitions = Lists.newArrayList();
for (String partition : partitions) {
- if (!shouldExistPartitionNames.contains(partition)) {
+ if (shouldExistPartitionNames.contains(partition)) {
+ if (existPartitionNames.contains(partition)) {
+ // regenerated name equals the stored physical name
(deterministic naming)
+ resolvedPartitions.add(partition);
+ continue;
+ }
+ // The partition is addressed by its regenerated (SHA)
name. If a physical partition with
+ // the same descriptor already exists under a legacy name,
remap to it; otherwise the
+ // alias is a not-yet-created partition that
alignMvPartition will materialize.
+ PartitionKeyDesc generatedDesc =
relatedPartitionDescs.stream()
Review Comment:
[P2] Index regenerated aliases before resolving the request
This rescans every `relatedPartitionDescs` entry for each valid regenerated
alias that is not already a physical name (the legacy time-suffixed and
not-yet-aligned cases this branch supports). A bulk request over 10,000 such
aliases therefore performs about 50 million descriptor serializations/name
generations, including repeated SHA-256 work for long names, while the MTMV and
all PCT tables remain read-locked. Build a generated-name-to-descriptor map
alongside `shouldExistPartitionNames` and resolve each alias in O(1);
constructing that map is also the right place to reject any duplicate generated
name.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVCache.java:
##########
@@ -131,12 +132,15 @@ public static MTMVCache from(String defSql,
CascadesContext cascadesContext = planner.getCascadesContext();
Plan rewritePlan = cascadesContext.getRewritePlan();
- // Only add SessionVarGuardExpr if requested
- Optional<SessionVarGuardRewriter> exprRewriter = addSessionVarGuard
- ? Optional.of(new SessionVarGuardRewriter(
+ // Only add SessionVarGuardExpr for the families selected by
guardMask. The mask comes from the
+ // query session that first needs this cache; the guard content is
independent of the session the
+ // cache is generated in, so a cache built in the creation zone
(background refresh) still carries
+ // the guards and stays effective for a cross-zone query.
+ Optional<SessionVarGuardRewriter> exprRewriter = guardMask ==
SessionVarGuardRewriter.GUARD_NONE
+ ? Optional.empty()
+ : Optional.of(new SessionVarGuardRewriter(
Review Comment:
[P1] Keep cache guards distinct from nested view guards
With `pre_materialized_view_rewrite_strategy='FORCE_IN_RBO'`, a view and
this MTMV both created in UTC produce the same guarded tree for a `+08:00`
query:
`Project(guard[UTC](date_trunc(ts, 'day'))) -> Scan(t)`
`BindRelation` adds the query-side guard for the view; cache analysis
expands that view in UTC (so it adds none) and this rewriter adds the identical
outer guard from the MTMV map. `RecordPlanForMvPreRewrite` snapshots the query
guard before the final `MergeGuardExpr` batch, and `SessionVarGuardExpr.equals`
compares only child plus map, so pre-RBO StructInfo matching can substitute the
MTMV. The no-rewrite path later drops the guard and evaluates in +08, but the
substituted path reads the UTC-refreshed value; for `2024-01-01 20:30Z`, those
are local January 2 midnight versus January 1 08:00. Please make a cache
mismatch guard structurally distinct from a nested persisted-object guard (or
reject matching when the relevant guard family is present), and add a
`FORCE_IN_RBO` UTC-view -> UTC-MTMV cross-zone rewrite-failure/result test.
--
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]
