github-actions[bot] commented on code in PR #66307:
URL: https://github.com/apache/doris/pull/66307#discussion_r3885789210
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/PhysicalProperties.java:
##########
@@ -59,26 +60,39 @@ public class PhysicalProperties {
private final DistributionSpec distributionSpec;
+ private final Optional<NaturalDistributionMappingSpec>
naturalDistributionMappingSpec;
+
private Integer hashCode = null;
private PhysicalProperties() {
- this.orderSpec = new OrderSpec();
- this.distributionSpec = DistributionSpecAny.INSTANCE;
+ this(DistributionSpecAny.INSTANCE, new OrderSpec(), Optional.empty());
}
public PhysicalProperties(DistributionSpec distributionSpec) {
- this.distributionSpec = distributionSpec;
- this.orderSpec = new OrderSpec();
+ this(distributionSpec, new OrderSpec(),
naturalMappingSpecFrom(distributionSpec));
Review Comment:
[P1] Stop hidden locality at runtime placement barriers
These constructors now recreate `NaturalDistributionMappingSpec` from a
retained natural hash spec, but several reachable operators retain that proof
even though their executable boundary changes or stops truthfully reporting
storage bucket-to-task placement: Generate (TableFunction PASSTHROUGH/NOOP),
PartitionTopN/Window (PartitionSort PASSTHROUGH and Analytic NOOP), ordinary
NLJ (ADAPTIVE_PASSTHROUGH), and serial/force-passthrough broadcast HashJoin. A
following Project can hide `k1` while retaining `M(d1)->bucket[0]`; an outer
mapping join then requests BUCKET_HASH_SHUFFLE using the Project's empty
distribute-expression list. The empty partitioner sends that side to one local
channel while its peer remains bucket-aligned, so matching rows can be missed.
Please retain hidden locality only across runtime boundaries that truthfully
preserve BUCKET placement (or strip both natural and hash-spec mappings there),
and add multi-task result regressions for these operator families.
##########
fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java:
##########
@@ -1063,6 +1067,28 @@ private void checkAndPrepareMeta() {
setState(RestoreJobState.CREATING);
}
+ private boolean validateDistributionMappingConstraintsForRestore() {
+ boolean featureCompatibilityValidated = false;
+ for (String tableName : jobInfo.backupOlapTableObjects.keySet()) {
+ OlapTable restoredTable = (OlapTable)
backupMeta.getTable(tableName);
+ if
(env.getConstraintManager().getDistributionMappingConstraints(restoredTable).isEmpty())
{
+ continue;
+ }
+ try {
+ if (!featureCompatibilityValidated) {
+
env.getConstraintManager().validateDistributionMappingFeatureCompatibility();
+ featureCompatibilityValidated = true;
+ }
+
env.getConstraintManager().validateDistributionMappingConstraints(restoredTable);
Review Comment:
[P1] Validate restored mapping names against destination constraints
This validates each backup mapping only against the backup table's schema
and FE compatibility. A live sequence of backup mapping `c` -> drop `c` -> add
centralized PK/UK `c` -> atomic restore then publishes the backup's table-local
`c` while the name-keyed centralized `c` remains. Both ordinary ADD paths
forbid that state, and afterward `SHOW CONSTRAINTS` and mapping-enabled
planning hit the explicit collision exception. Please validate restored
mappings against the destination constraint namespace under the restore
publication fence (or durably define which namespace survives), with this
backup/drop/add/restore case covered.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java:
##########
@@ -101,6 +113,30 @@ public void run(ConnectContext ctx, StmtExecutor executor)
throws Exception {
String.format("after drop constraint %s on table %s",
constraint.getName(), tableNameInfo));
}
+ private void dropDistributionMapping(TableNameInfo tableNameInfo,
+ TableIf analyzedTable) throws Exception {
+ EditLog.EditLogItem logItem;
+ analyzedTable.getDatabase().readLock();
+ try {
+ if
(analyzedTable.getDatabase().getCatalog().getDbNullable(tableNameInfo.getDb())
+ != analyzedTable.getDatabase()
+ ||
analyzedTable.getDatabase().getTableNullable(tableNameInfo.getTbl()) !=
analyzedTable) {
+ throw new AnalysisException("Table changed while dropping
constraint on " + tableNameInfo);
+ }
+ analyzedTable.writeLock();
+ try {
+ logItem = Env.getCurrentEnv().getConstraintManager()
Review Comment:
[P1] Reject mapping DROP during atomic restore
Atomic restore marks the live target with `inAtomicRestore` specifically to
fence ALTER operations, and the new mapping ADD calls
`checkNormalStateForAlter()` under this same table lock. DROP skips that check.
While the restored copy is staged, `DROP CONSTRAINT m` can therefore journal
and return success against the live object; `atomicReplaceOlapTables` later
replaces it with the backup-derived object that still has `m`, silently
reversing a successful DDL. Please apply the same state check before the DROP
mutation and cover a paused atomic restore through completion and cancellation.
--
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]
