github-actions[bot] commented on code in PR #66795:
URL: https://github.com/apache/doris/pull/66795#discussion_r3841906429
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/RefreshMTMVInfo.java:
##########
@@ -97,18 +97,76 @@ 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();
+ // Index every related partition desc by its generated
(regenerated/alias) name so each requested
+ // alias below is resolved in O(1). Rescanning
relatedPartitionDescs for every alias would
+ // re-serialize every descriptor (including SHA-256 work for long
names) per alias, which is
+ // O(n * m) while the MTMV and all PCT tables stay read-locked.
Constructing the index is also
+ // the right place to reject duplicate generated names: two
distinct descriptors mapping to the
+ // same name would make the alias ambiguous.
+ Map<String, PartitionKeyDesc> generatedNameToDesc =
Maps.newHashMap();
+ for (PartitionKeyDesc desc : relatedPartitionDescs) {
+ String generatedName =
MTMVPartitionUtil.generatePartitionName(desc);
+ PartitionKeyDesc previous =
generatedNameToDesc.putIfAbsent(generatedName, desc);
+ if (previous != null) {
+ throw new org.apache.doris.common.AnalysisException(
+ "duplicate generated partition name: " +
generatedName);
+ }
+ }
+ Set<String> shouldExistPartitionNames =
generatedNameToDesc.keySet();
+ // 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.
+ // partition is in shouldExistPartitionNames (the map's
key set), so the lookup is
+ // guaranteed to succeed.
+ String physicalName =
descToPhysicalName.get(generatedNameToDesc.get(partition));
+ resolvedPartitions.add(physicalName != null ? physicalName
: partition);
Review Comment:
[P2] Deduplicate aliases after resolving them to physical names
If a legacy descriptor is stored as `legacy_p` and its regenerated alias is
`sha_p`, `REFRESH ... PARTITIONS(legacy_p, sha_p)` resolves both entries to
`legacy_p`. The task keeps that duplicate list: with the default
`refresh_partition_num=1` it runs two separate INSERT OVERWRITEs for the same
partition, and with a larger batch execution is deduplicated only by the
per-batch set while `generateRefreshMode` still counts both entries (so one
distinct target in a two-partition MV can be reported as `COMPLETE`). Please
resolve into an insertion-ordered set, or reject duplicate logical targets, and
cover the alias-plus-physical case in a 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]
