Skip to content

Commit 28be8c7

Browse files
[fix](pkfk) Fix drop table not drop constraint related info (#58958)
Problem Summary: Create two tables, t1 and t2. Create a primary key on table t1 and a foreign key on table t2 referencing t1 primary key. Then drop table t2, and then try to delete the primary key on t1. doris will report error: errCode = 2, detailMessage = Can not find table XXXX in constraint. This is because the `foreignTables` metadata in the `PrimaryKeyConstraint` was not updated when the foreign key table was dropped. `foreignTables` stores the table IDs of the foreign key tables that reference the primary key. When dropping the foreign key table or drop the foreign key, the corresponding entries in `foreignTables` should also be removed.
1 parent 0a5c352 commit 28be8c7

File tree

7 files changed

+143
-14
lines changed

7 files changed

+143
-14
lines changed

fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@
2121
import org.apache.doris.catalog.constraint.Constraint;
2222
import org.apache.doris.catalog.constraint.ForeignKeyConstraint;
2323
import org.apache.doris.catalog.constraint.PrimaryKeyConstraint;
24+
import org.apache.doris.catalog.constraint.TableIdentifier;
2425
import org.apache.doris.catalog.constraint.UniqueConstraint;
2526
import org.apache.doris.cluster.ClusterNamespace;
2627
import org.apache.doris.common.DdlException;
2728
import org.apache.doris.common.MetaNotFoundException;
2829
import org.apache.doris.common.Pair;
30+
import org.apache.doris.common.util.MetaLockUtils;
2931
import org.apache.doris.datasource.systable.SysTable;
3032
import org.apache.doris.info.TableValuedFunctionRefInfo;
3133
import org.apache.doris.nereids.exceptions.AnalysisException;
@@ -47,6 +49,7 @@
4749
import java.io.DataOutput;
4850
import java.io.IOException;
4951
import java.util.Collections;
52+
import java.util.Comparator;
5053
import java.util.List;
5154
import java.util.Map;
5255
import java.util.Map.Entry;
@@ -353,6 +356,56 @@ default void replayDropConstraint(String name) {
353356
dropConstraint(name, true);
354357
}
355358

359+
// when table has foreign key constraint referencing to primary key of other table,
360+
// need to remove this table identifier from primary table's foreign table set when drop this
361+
// when table has primary key constraint, when drop table(this), need to remove the foreign key referenced this
362+
default void removeTableIdentifierFromPrimaryTable() {
363+
Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
364+
for (Constraint constraint : constraintMap.values()) {
365+
dropConstraintRefWithLock(constraint);
366+
}
367+
}
368+
369+
default void dropConstraintRefWithLock(Constraint constraint) {
370+
List<TableIf> tables = getConstraintRelatedTables(constraint);
371+
tables.sort((Comparator.comparing(TableIf::getId)));
372+
MetaLockUtils.writeLockTables(tables);
373+
try {
374+
dropConstraintRef(constraint);
375+
} finally {
376+
MetaLockUtils.writeUnlockTables(tables);
377+
}
378+
}
379+
380+
default void dropConstraintRef(Constraint constraint) {
381+
if (constraint instanceof PrimaryKeyConstraint) {
382+
((PrimaryKeyConstraint) constraint).getForeignTables()
383+
.forEach(t -> t.dropFKReferringPK(this, (PrimaryKeyConstraint) constraint));
384+
} else if (constraint instanceof ForeignKeyConstraint) {
385+
ForeignKeyConstraint foreignKeyConstraint = (ForeignKeyConstraint) constraint;
386+
Optional<TableIf> primaryTableIf = foreignKeyConstraint.getReferencedTableOrNull();
387+
if (primaryTableIf.isPresent()) {
388+
Map<String, Constraint> refTableConstraintMap = primaryTableIf.get().getConstraintsMapUnsafe();
389+
for (Constraint refTableConstraint : refTableConstraintMap.values()) {
390+
if (refTableConstraint instanceof PrimaryKeyConstraint) {
391+
PrimaryKeyConstraint primaryKeyConstraint = (PrimaryKeyConstraint) refTableConstraint;
392+
primaryKeyConstraint.removeForeignTable(new TableIdentifier(this));
393+
}
394+
}
395+
}
396+
}
397+
}
398+
399+
default List<TableIf> getConstraintRelatedTables(Constraint constraint) {
400+
List<TableIf> tables = Lists.newArrayList();
401+
if (constraint instanceof PrimaryKeyConstraint) {
402+
tables.addAll(((PrimaryKeyConstraint) constraint).getForeignTables());
403+
} else if (constraint instanceof ForeignKeyConstraint) {
404+
tables.add(((ForeignKeyConstraint) constraint).getReferencedTable());
405+
}
406+
return tables;
407+
}
408+
356409
default void dropConstraint(String name, boolean replay) {
357410
Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
358411
if (!constraintMap.containsKey(name)) {
@@ -361,10 +414,7 @@ default void dropConstraint(String name, boolean replay) {
361414
}
362415
Constraint constraint = constraintMap.get(name);
363416
constraintMap.remove(name);
364-
if (constraint instanceof PrimaryKeyConstraint) {
365-
((PrimaryKeyConstraint) constraint).getForeignTables()
366-
.forEach(t -> t.dropFKReferringPK(this, (PrimaryKeyConstraint) constraint));
367-
}
417+
dropConstraintRefWithLock(constraint);
368418
if (!replay) {
369419
Env.getCurrentEnv().getEditLog().logDropConstraint(new AlterConstraintLog(constraint, this));
370420
}

fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/ForeignKeyConstraint.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.List;
3030
import java.util.Map;
3131
import java.util.Objects;
32+
import java.util.Optional;
3233
import java.util.Set;
3334

3435
public class ForeignKeyConstraint extends Constraint {
@@ -90,6 +91,16 @@ public TableIf getReferencedTable() {
9091
return referencedTable.toTableIf();
9192
}
9293

94+
public Optional<TableIf> getReferencedTableOrNull() {
95+
TableIf res = null;
96+
try {
97+
res = referencedTable.toTableIf();
98+
} catch (Exception ignored) {
99+
// do nothing
100+
}
101+
return Optional.ofNullable(res);
102+
}
103+
93104
public Boolean isReferringPK(TableIf table, PrimaryKeyConstraint constraint) {
94105
return constraint.getPrimaryKeyNames().equals(getPrimaryKeyNames())
95106
&& getReferencedTable().equals(table);

fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/PrimaryKeyConstraint.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ public List<TableIf> getForeignTables() {
6060
.collect(ImmutableList.toImmutableList());
6161
}
6262

63+
public void removeForeignTable(TableIdentifier tableIdentifier) {
64+
foreignTables.remove(tableIdentifier);
65+
}
66+
6367
@Override
6468
public String toString() {
6569
return "PRIMARY KEY (" + String.join(", ", columns) + ")";

fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/TableIdentifier.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public TableIf toTableIf() {
5656
}
5757
TableIf tableIf = databaseIf.getTableNullable(tableId);
5858
if (tableIf == null) {
59-
throw new AnalysisException(String.format("Can not find table %s in constraint", databaseId));
59+
throw new AnalysisException(String.format("Can not find table %s in constraint", tableId));
6060
}
6161
return tableIf;
6262
}

fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,6 +1020,7 @@ public boolean unprotectDropTable(Database db, Table table, boolean isForceDrop,
10201020
Env.getCurrentEnv().getAnalysisManager().removeTableStats(table.getId());
10211021
Env.getCurrentEnv().getDictionaryManager().dropTableDictionaries(db.getName(), table.getName());
10221022
Env.getCurrentEnv().getQueryStats().clear(Env.getCurrentInternalCatalog().getId(), db.getId(), table.getId());
1023+
table.removeTableIdentifierFromPrimaryTable();
10231024
db.unregisterTable(table.getId());
10241025
StopWatch watch = StopWatch.createStarted();
10251026
Env.getCurrentRecycleBin().recycleTable(db.getId(), table, isReplay, isForceDrop, recycleTime);

fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919

2020
import org.apache.doris.catalog.TableIf;
2121
import org.apache.doris.catalog.constraint.Constraint;
22+
import org.apache.doris.catalog.constraint.ForeignKeyConstraint;
2223
import org.apache.doris.catalog.constraint.PrimaryKeyConstraint;
23-
import org.apache.doris.common.util.MetaLockUtils;
2424
import org.apache.doris.nereids.NereidsPlanner;
2525
import org.apache.doris.nereids.exceptions.AnalysisException;
2626
import org.apache.doris.nereids.properties.PhysicalProperties;
@@ -37,7 +37,6 @@
3737
import org.apache.logging.log4j.LogManager;
3838
import org.apache.logging.log4j.Logger;
3939

40-
import java.util.Comparator;
4140
import java.util.List;
4241
import java.util.Set;
4342

@@ -59,29 +58,34 @@ public DropConstraintCommand(String name, LogicalPlan plan) {
5958
this.plan = plan;
6059
}
6160

61+
private static List<TableIf> getConstraintRelatedTables(Constraint constraint) {
62+
List<TableIf> tables = Lists.newArrayList();
63+
if (constraint instanceof PrimaryKeyConstraint) {
64+
tables.addAll(((PrimaryKeyConstraint) constraint).getForeignTables());
65+
} else if (constraint instanceof ForeignKeyConstraint) {
66+
tables.add(((ForeignKeyConstraint) constraint).getReferencedTable());
67+
}
68+
return tables;
69+
}
70+
6271
@Override
6372
public void run(ConnectContext ctx, StmtExecutor executor) throws Exception {
6473
TableIf table = extractTable(ctx, plan);
65-
List<TableIf> tables = Lists.newArrayList(table);
6674
table.readLock();
6775
try {
6876
Constraint constraint = table.getConstraintsMapUnsafe().get(name);
6977
if (constraint == null) {
7078
throw new AnalysisException(
7179
String.format("Unknown constraint %s on table %s.", name, table.getName()));
7280
}
73-
if (constraint instanceof PrimaryKeyConstraint) {
74-
tables.addAll(((PrimaryKeyConstraint) constraint).getForeignTables());
75-
}
7681
} finally {
7782
table.readUnlock();
7883
}
79-
tables.sort((Comparator.comparing(TableIf::getId)));
80-
MetaLockUtils.writeLockTables(tables);
84+
table.writeLock();
8185
try {
8286
table.dropConstraint(name, false);
8387
} finally {
84-
MetaLockUtils.writeUnlockTables(tables);
88+
table.writeUnlock();
8589
}
8690
}
8791

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
suite("test_pk_fk_drop_table") {
19+
// case 1
20+
multi_sql """drop table if exists store_sales_test;
21+
drop table if exists customer_test;
22+
create table customer_test(c_customer_sk int,c_customer_id int);
23+
CREATE TABLE `store_sales_test` (
24+
`ss_customer_sk` int NULL,
25+
`d_date` date NULL
26+
) ENGINE=OLAP
27+
DUPLICATE KEY(`ss_customer_sk`, `d_date`)
28+
DISTRIBUTED BY RANDOM BUCKETS AUTO
29+
PROPERTIES (
30+
"replication_allocation" = "tag.location.default: 1"
31+
);
32+
alter table customer_test add constraint c_pk primary key(c_customer_sk);
33+
alter table store_sales_test add constraint ss_c_fk foreign key(ss_customer_sk) references customer_test(c_customer_sk);
34+
drop table store_sales_test;"""
35+
// expect: successful drop
36+
sql "alter table customer_test drop constraint c_pk;"
37+
38+
multi_sql """
39+
drop table if exists store_sales_test;
40+
drop table if exists customer_test;
41+
create table customer_test(c_customer_sk int,c_customer_id int) PROPERTIES (
42+
"replication_allocation" = "tag.location.default: 1"
43+
);
44+
CREATE TABLE `store_sales_test` (
45+
`ss_customer_sk` int NULL,
46+
`d_date` date NULL
47+
) ENGINE=OLAP
48+
DUPLICATE KEY(`ss_customer_sk`, `d_date`)
49+
DISTRIBUTED BY RANDOM BUCKETS AUTO
50+
PROPERTIES (
51+
"replication_allocation" = "tag.location.default: 1"
52+
);
53+
alter table customer_test add constraint c_pk primary key(c_customer_sk);
54+
alter table store_sales_test add constraint ss_c_fk foreign key(ss_customer_sk) references customer_test(c_customer_sk);
55+
drop table customer_test;
56+
"""
57+
// expect: not throw
58+
sql "show constraints from store_sales_test;"
59+
}

0 commit comments

Comments
 (0)