This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git
commit 7d9313c8079cb60b23ae17da3cc81e14e54444de
Author: 谢健 <jianx...@gmail.com>
AuthorDate: Tue Jan 30 19:35:12 2024 +0800
[revert](Nereids): revert#30578 #30422 (#30594)
* Revert "[fix](Nereids): don't log edit log when replaying"
* Revert "[fix](Nereids) create constraint write edit log in lock scope
(#30422)"
This reverts commit 27a12d37acbe2ca807f7ce56125ad4773d89f584.
---
.../java/org/apache/doris/catalog/TableIf.java | 78 ++++++++++------------
.../trees/plans/commands/AddConstraintCommand.java | 13 ++--
.../plans/commands/DropConstraintCommand.java | 5 +-
.../java/org/apache/doris/persist/EditLog.java | 2 +-
.../catalog/constraint/ConstraintPersistTest.java | 7 +-
5 files changed, 50 insertions(+), 55 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java
index 612bfc65eab..67efd98fec6 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java
@@ -26,7 +26,6 @@ import org.apache.doris.cluster.ClusterNamespace;
import org.apache.doris.common.DdlException;
import org.apache.doris.common.MetaNotFoundException;
import org.apache.doris.nereids.exceptions.AnalysisException;
-import org.apache.doris.persist.AlterConstraintLog;
import org.apache.doris.statistics.AnalysisInfo;
import org.apache.doris.statistics.BaseAnalysisTask;
import org.apache.doris.statistics.ColumnStatistic;
@@ -226,7 +225,7 @@ public interface TableIf {
}
// Note this function is not thread safe
- default void checkConstraintNotExistenceUnsafe(String name, Constraint
primaryKeyConstraint,
+ default void checkConstraintNotExistence(String name, Constraint
primaryKeyConstraint,
Map<String, Constraint> constraintMap) {
if (constraintMap.containsKey(name)) {
throw new RuntimeException(String.format("Constraint name %s has
existed", name));
@@ -239,72 +238,63 @@ public interface TableIf {
}
}
- default void addUniqueConstraint(String name, ImmutableList<String>
columns, boolean replay) {
+ default Constraint addUniqueConstraint(String name, ImmutableList<String>
columns) {
writeLock();
try {
Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
UniqueConstraint uniqueConstraint = new UniqueConstraint(name,
ImmutableSet.copyOf(columns));
- checkConstraintNotExistenceUnsafe(name, uniqueConstraint,
constraintMap);
+ checkConstraintNotExistence(name, uniqueConstraint, constraintMap);
constraintMap.put(name, uniqueConstraint);
- if (!replay) {
- Env.getCurrentEnv().getEditLog().logAddConstraint(
- new AlterConstraintLog(uniqueConstraint, this));
- }
+ return uniqueConstraint;
} finally {
writeUnlock();
}
}
- default void addPrimaryKeyConstraint(String name, ImmutableList<String>
columns, boolean replay) {
+ default Constraint addPrimaryKeyConstraint(String name,
ImmutableList<String> columns) {
writeLock();
try {
Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
PrimaryKeyConstraint primaryKeyConstraint = new
PrimaryKeyConstraint(name, ImmutableSet.copyOf(columns));
- checkConstraintNotExistenceUnsafe(name, primaryKeyConstraint,
constraintMap);
+ checkConstraintNotExistence(name, primaryKeyConstraint,
constraintMap);
constraintMap.put(name, primaryKeyConstraint);
- if (!replay) {
- Env.getCurrentEnv().getEditLog().logAddConstraint(
- new AlterConstraintLog(primaryKeyConstraint, this));
- }
+ return primaryKeyConstraint;
} finally {
writeUnlock();
}
}
- default PrimaryKeyConstraint tryGetPrimaryKeyForForeignKeyUnsafe(
- PrimaryKeyConstraint requirePrimaryKey, TableIf referencedTable) {
- Optional<Constraint> primaryKeyConstraint =
referencedTable.getConstraintsMapUnsafe().values().stream()
- .filter(requirePrimaryKey::equals)
- .findFirst();
- if (!primaryKeyConstraint.isPresent()) {
- throw new AnalysisException(String.format(
- "Foreign key constraint requires a primary key constraint
%s in %s",
- requirePrimaryKey.getPrimaryKeyNames(),
referencedTable.getName()));
+ default void updatePrimaryKeyForForeignKey(PrimaryKeyConstraint
requirePrimaryKey, TableIf referencedTable) {
+ referencedTable.writeLock();
+ try {
+ Optional<Constraint> primaryKeyConstraint =
referencedTable.getConstraintsMapUnsafe().values().stream()
+ .filter(requirePrimaryKey::equals)
+ .findFirst();
+ if (!primaryKeyConstraint.isPresent()) {
+ throw new AnalysisException(String.format(
+ "Foreign key constraint requires a primary key
constraint %s in %s",
+ requirePrimaryKey.getPrimaryKeyNames(),
referencedTable.getName()));
+ }
+ ((PrimaryKeyConstraint)
(primaryKeyConstraint.get())).addForeignTable(this);
+ } finally {
+ referencedTable.writeUnlock();
}
- return ((PrimaryKeyConstraint) (primaryKeyConstraint.get()));
}
- default void addForeignConstraint(String name, ImmutableList<String>
columns,
- TableIf referencedTable, ImmutableList<String> referencedColumns,
boolean replay) {
+ default Constraint addForeignConstraint(String name, ImmutableList<String>
columns,
+ TableIf referencedTable, ImmutableList<String> referencedColumns) {
writeLock();
- referencedTable.writeLock();
try {
Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
ForeignKeyConstraint foreignKeyConstraint =
new ForeignKeyConstraint(name, columns, referencedTable,
referencedColumns);
- checkConstraintNotExistenceUnsafe(name, foreignKeyConstraint,
constraintMap);
- PrimaryKeyConstraint requirePrimaryKeyName = new
PrimaryKeyConstraint(name,
+ checkConstraintNotExistence(name, foreignKeyConstraint,
constraintMap);
+ PrimaryKeyConstraint requirePrimaryKey = new
PrimaryKeyConstraint(name,
foreignKeyConstraint.getReferencedColumnNames());
- PrimaryKeyConstraint primaryKeyConstraint =
- tryGetPrimaryKeyForForeignKeyUnsafe(requirePrimaryKeyName,
referencedTable);
- primaryKeyConstraint.addForeignTable(this);
+ updatePrimaryKeyForForeignKey(requirePrimaryKey, referencedTable);
constraintMap.put(name, foreignKeyConstraint);
- if (!replay) {
- Env.getCurrentEnv().getEditLog().logAddConstraint(
- new AlterConstraintLog(foreignKeyConstraint, this));
- }
+ return foreignKeyConstraint;
} finally {
- referencedTable.writeUnlock();
writeUnlock();
}
}
@@ -313,21 +303,22 @@ public interface TableIf {
if (constraint instanceof UniqueConstraint) {
UniqueConstraint uniqueConstraint = (UniqueConstraint) constraint;
this.addUniqueConstraint(constraint.getName(),
-
ImmutableList.copyOf(uniqueConstraint.getUniqueColumnNames()), true);
+
ImmutableList.copyOf(uniqueConstraint.getUniqueColumnNames()));
} else if (constraint instanceof PrimaryKeyConstraint) {
PrimaryKeyConstraint primaryKeyConstraint = (PrimaryKeyConstraint)
constraint;
this.addPrimaryKeyConstraint(primaryKeyConstraint.getName(),
-
ImmutableList.copyOf(primaryKeyConstraint.getPrimaryKeyNames()), true);
+
ImmutableList.copyOf(primaryKeyConstraint.getPrimaryKeyNames()));
} else if (constraint instanceof ForeignKeyConstraint) {
ForeignKeyConstraint foreignKey = (ForeignKeyConstraint)
constraint;
this.addForeignConstraint(foreignKey.getName(),
ImmutableList.copyOf(foreignKey.getForeignKeyNames()),
foreignKey.getReferencedTable(),
-
ImmutableList.copyOf(foreignKey.getReferencedColumnNames()), true);
+
ImmutableList.copyOf(foreignKey.getReferencedColumnNames()));
}
}
- default void dropConstraint(String name, boolean replay) {
+ default Constraint dropConstraint(String name) {
+ Constraint dropConstraint;
writeLock();
try {
Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
@@ -341,12 +332,11 @@ public interface TableIf {
((PrimaryKeyConstraint) constraint).getForeignTables()
.forEach(t -> t.dropFKReferringPK(this,
(PrimaryKeyConstraint) constraint));
}
- if (!replay) {
- Env.getCurrentEnv().getEditLog().logDropConstraint(new
AlterConstraintLog(constraint, this));
- }
+ dropConstraint = constraint;
} finally {
writeUnlock();
}
+ return dropConstraint;
}
default void dropFKReferringPK(TableIf table, PrimaryKeyConstraint
constraint) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AddConstraintCommand.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AddConstraintCommand.java
index 38bf71133c0..8c90bc0f914 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AddConstraintCommand.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AddConstraintCommand.java
@@ -17,6 +17,7 @@
package org.apache.doris.nereids.trees.plans.commands;
+import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.TableIf;
import org.apache.doris.common.Pair;
import org.apache.doris.nereids.NereidsPlanner;
@@ -29,6 +30,7 @@ import
org.apache.doris.nereids.trees.plans.commands.ExplainCommand.ExplainLevel
import org.apache.doris.nereids.trees.plans.logical.LogicalCatalogRelation;
import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
+import org.apache.doris.persist.AlterConstraintLog;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.qe.StmtExecutor;
@@ -61,16 +63,19 @@ public class AddConstraintCommand extends Command
implements ForwardWithSync {
@Override
public void run(ConnectContext ctx, StmtExecutor executor) throws
Exception {
Pair<ImmutableList<String>, TableIf> columnsAndTable =
extractColumnsAndTable(ctx, constraint.toProject());
+ org.apache.doris.catalog.constraint.Constraint catalogConstraint =
null;
if (constraint.isForeignKey()) {
Pair<ImmutableList<String>, TableIf> referencedColumnsAndTable
= extractColumnsAndTable(ctx,
constraint.toReferenceProject());
- columnsAndTable.second.addForeignConstraint(name,
columnsAndTable.first,
- referencedColumnsAndTable.second,
referencedColumnsAndTable.first, false);
+ catalogConstraint =
columnsAndTable.second.addForeignConstraint(name, columnsAndTable.first,
+ referencedColumnsAndTable.second,
referencedColumnsAndTable.first);
} else if (constraint.isPrimaryKey()) {
- columnsAndTable.second.addPrimaryKeyConstraint(name,
columnsAndTable.first, false);
+ catalogConstraint =
columnsAndTable.second.addPrimaryKeyConstraint(name, columnsAndTable.first);
} else if (constraint.isUnique()) {
- columnsAndTable.second.addUniqueConstraint(name,
columnsAndTable.first, false);
+ catalogConstraint =
columnsAndTable.second.addUniqueConstraint(name, columnsAndTable.first);
}
+ Env.getCurrentEnv().getEditLog().logAddConstraint(
+ new AlterConstraintLog(catalogConstraint,
columnsAndTable.second));
}
private Pair<ImmutableList<String>, TableIf>
extractColumnsAndTable(ConnectContext ctx, LogicalPlan plan) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java
index 9878b0e9263..84143f234c3 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java
@@ -17,6 +17,7 @@
package org.apache.doris.nereids.trees.plans.commands;
+import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.TableIf;
import org.apache.doris.nereids.NereidsPlanner;
import org.apache.doris.nereids.exceptions.AnalysisException;
@@ -27,6 +28,7 @@ import
org.apache.doris.nereids.trees.plans.commands.ExplainCommand.ExplainLevel
import org.apache.doris.nereids.trees.plans.logical.LogicalCatalogRelation;
import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
+import org.apache.doris.persist.AlterConstraintLog;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.qe.StmtExecutor;
@@ -56,7 +58,8 @@ public class DropConstraintCommand extends Command implements
ForwardWithSync {
@Override
public void run(ConnectContext ctx, StmtExecutor executor) throws
Exception {
TableIf table = extractTable(ctx, plan);
- table.dropConstraint(name, false);
+ org.apache.doris.catalog.constraint.Constraint catalogConstraint =
table.dropConstraint(name);
+ Env.getCurrentEnv().getEditLog().logDropConstraint(new
AlterConstraintLog(catalogConstraint, table));
}
private TableIf extractTable(ConnectContext ctx, LogicalPlan plan) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
b/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
index 8400c0d6672..958277dd6b7 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
@@ -983,7 +983,7 @@ public class EditLog {
}
case OperationType.OP_DROP_CONSTRAINT: {
final AlterConstraintLog log = (AlterConstraintLog)
journal.getData();
-
log.getTableIf().dropConstraint(log.getConstraint().getName(), true);
+
log.getTableIf().dropConstraint(log.getConstraint().getName());
break;
}
case OperationType.OP_ALTER_USER: {
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintPersistTest.java
b/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintPersistTest.java
index 045d1893022..64f3db583ad 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintPersistTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintPersistTest.java
@@ -169,11 +169,8 @@ class ConstraintPersistTest extends TestWithFeService
implements PlanPatternMatc
@Test
void externalTableTest() throws Exception {
ExternalTable externalTable = new ExternalTable();
- try {
- externalTable.addPrimaryKeyConstraint("pk",
ImmutableList.of("col"), false);
- } catch (Exception ignore) {
- // ignore
- }
+ externalTable.addPrimaryKeyConstraint("pk", ImmutableList.of("col"));
+
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
DataOutput output = new DataOutputStream(outputStream);
externalTable.write(output);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
