🌐 AI搜索 & 代理 主页
Skip to content

Commit 2d5fe51

Browse files
alvherrealvherre
committed
Fix some more bugs in foreign keys connecting partitioned tables
* In DetachPartitionFinalize() we were applying a tuple conversion map to tuples that didn't need one, which can lead to erratic behavior if a partitioned table has a partition with a different column order, as reported by Alexander Lakhin. This was introduced by 53af949. Don't do that. Also, modify a recently added test case to exercise this. * The same function as well as CloneFkReferenced() were acquiring AccessShareLock on a partition, only to have CreateTrigger() later acquire ShareRowExclusiveLock on it. This can lead to deadlock by lock escalation, unnecessarily. Avoid that by acquiring the stronger lock to begin with. This probably dates back to branch 12, but I have never seen a report of this being a problem in the field. * Innocuous but wasteful: also introduced by 53af949, we were reading a pg_constraint tuple from syscache that we don't need, as reported by Tender Wang. Don't. Backpatch to 15. Discussion: https://postgr.es/m/461e9c26-2076-8224-e119-84998b6a784e@gmail.com
1 parent 2845cd1 commit 2d5fe51

File tree

3 files changed

+27
-30
lines changed

3 files changed

+27
-30
lines changed

‎src/backend/commands/tablecmds.c‎

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10372,6 +10372,9 @@ addFkRecurseReferenced(Constraint *fkconstraint, Relation rel,
1037210372
Oid deleteTriggerOid,
1037310373
updateTriggerOid;
1037410374

10375+
Assert(CheckRelationLockedByMe(pkrel, ShareRowExclusiveLock, true));
10376+
Assert(CheckRelationLockedByMe(rel, ShareRowExclusiveLock, true));
10377+
1037510378
/*
1037610379
* Create the action triggers that enforce the constraint.
1037710380
*/
@@ -10398,6 +10401,7 @@ addFkRecurseReferenced(Constraint *fkconstraint, Relation rel,
1039810401
Oid partIndexId;
1039910402
ObjectAddress address;
1040010403

10404+
/* XXX would it be better to acquire these locks beforehand? */
1040110405
partRel = table_open(pd->oids[i], ShareRowExclusiveLock);
1040210406

1040310407
/*
@@ -10503,6 +10507,8 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel,
1050310507
updateTriggerOid;
1050410508

1050510509
Assert(OidIsValid(parentConstr));
10510+
Assert(CheckRelationLockedByMe(rel, ShareRowExclusiveLock, true));
10511+
Assert(CheckRelationLockedByMe(pkrel, ShareRowExclusiveLock, true));
1050610512

1050710513
if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
1050810514
ereport(ERROR,
@@ -10796,13 +10802,8 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
1079610802
continue;
1079710803
}
1079810804

10799-
/*
10800-
* Because we're only expanding the key space at the referenced side,
10801-
* we don't need to prevent any operation in the referencing table, so
10802-
* AccessShareLock suffices (assumes that dropping the constraint
10803-
* acquires AccessExclusiveLock).
10804-
*/
10805-
fkRel = table_open(constrForm->conrelid, AccessShareLock);
10805+
/* We need the same lock level that CreateTrigger will acquire */
10806+
fkRel = table_open(constrForm->conrelid, ShareRowExclusiveLock);
1080610807

1080710808
indexOid = constrForm->conindid;
1080810809
DeconstructFkConstraintRow(tuple,
@@ -19436,8 +19437,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
1943619437
foreach(cell, fks)
1943719438
{
1943819439
ForeignKeyCacheInfo *fk = lfirst(cell);
19439-
HeapTuple contup,
19440-
parentConTup;
19440+
HeapTuple contup;
1944119441
Form_pg_constraint conform;
1944219442
Oid insertTriggerOid,
1944319443
updateTriggerOid;
@@ -19455,13 +19455,6 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
1945519455
continue;
1945619456
}
1945719457

19458-
Assert(OidIsValid(conform->conparentid));
19459-
parentConTup = SearchSysCache1(CONSTROID,
19460-
ObjectIdGetDatum(conform->conparentid));
19461-
if (!HeapTupleIsValid(parentConTup))
19462-
elog(ERROR, "cache lookup failed for constraint %u",
19463-
conform->conparentid);
19464-
1946519458
/*
1946619459
* The constraint on this table must be marked no longer a child of
1946719460
* the parent's constraint, as do its check triggers.
@@ -19502,7 +19495,6 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
1950219495
Oid conffeqop[INDEX_MAX_KEYS];
1950319496
int numfkdelsetcols;
1950419497
AttrNumber confdelsetcols[INDEX_MAX_KEYS];
19505-
AttrMap *attmap;
1950619498
Relation refdRel;
1950719499

1950819500
DeconstructFkConstraintRow(contup,
@@ -19535,20 +19527,19 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
1953519527
fkconstraint->old_pktable_oid = InvalidOid;
1953619528
fkconstraint->location = -1;
1953719529

19538-
attmap = build_attrmap_by_name(RelationGetDescr(partRel),
19539-
RelationGetDescr(rel),
19540-
false);
19530+
/* set up colnames, used to generate the constraint name */
1954119531
for (int i = 0; i < numfks; i++)
1954219532
{
1954319533
Form_pg_attribute att;
1954419534

1954519535
att = TupleDescAttr(RelationGetDescr(partRel),
19546-
attmap->attnums[conkey[i] - 1] - 1);
19536+
conkey[i] - 1);
19537+
1954719538
fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs,
1954819539
makeString(NameStr(att->attname)));
1954919540
}
1955019541

19551-
refdRel = table_open(fk->confrelid, AccessShareLock);
19542+
refdRel = table_open(fk->confrelid, ShareRowExclusiveLock);
1955219543

1955319544
addFkRecurseReferenced(fkconstraint, partRel,
1955419545
refdRel,
@@ -19565,11 +19556,10 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
1956519556
true,
1956619557
InvalidOid, InvalidOid,
1956719558
conform->conperiod);
19568-
table_close(refdRel, AccessShareLock);
19559+
table_close(refdRel, NoLock); /* keep lock till end of xact */
1956919560
}
1957019561

1957119562
ReleaseSysCache(contup);
19572-
ReleaseSysCache(parentConTup);
1957319563
}
1957419564
list_free_deep(fks);
1957519565
if (trigrel)

‎src/test/regress/expected/foreign_key.out‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2944,17 +2944,20 @@ CREATE SCHEMA fkpart12
29442944
CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id)
29452945
CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd)
29462946
CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1)
2947-
CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2)
2947+
CREATE TABLE fk_p_1_2 (x int, y int, jd int NOT NULL, id int NOT NULL)
29482948
CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd)
29492949
CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1)
29502950
CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2)
2951-
CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL)
2951+
CREATE TABLE fk_r_1 ( p_jd int NOT NULL, x int, id int PRIMARY KEY, p_id int NOT NULL)
29522952
CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id)
29532953
CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1)
29542954
CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL,
29552955
FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd)
29562956
) PARTITION BY list (id);
29572957
SET search_path TO fkpart12;
2958+
ALTER TABLE fk_p_1_2 DROP COLUMN x, DROP COLUMN y;
2959+
ALTER TABLE fk_p_1 ATTACH PARTITION fk_p_1_2 FOR VALUES IN (2);
2960+
ALTER TABLE fk_r_1 DROP COLUMN x;
29582961
INSERT INTO fk_p VALUES (1, 1);
29592962
ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);
29602963
ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2);
@@ -2993,7 +2996,7 @@ Foreign-key constraints:
29932996
"fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd)
29942997
Number of partitions: 1 (Use \d+ to list them.)
29952998

2996-
INSERT INTO fk_r_1 VALUES (2, 1, 2); -- should fail
2999+
INSERT INTO fk_r_1 (id, p_id, p_jd) VALUES (2, 1, 2); -- should fail
29973000
ERROR: insert or update on table "fk_r_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey"
29983001
DETAIL: Key (p_id, p_jd)=(1, 2) is not present in table "fk_p".
29993002
DELETE FROM fk_p; -- should fail

‎src/test/regress/sql/foreign_key.sql‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2097,18 +2097,22 @@ CREATE SCHEMA fkpart12
20972097
CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id)
20982098
CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd)
20992099
CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1)
2100-
CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2)
2100+
CREATE TABLE fk_p_1_2 (x int, y int, jd int NOT NULL, id int NOT NULL)
21012101
CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd)
21022102
CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1)
21032103
CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2)
2104-
CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL)
2104+
CREATE TABLE fk_r_1 ( p_jd int NOT NULL, x int, id int PRIMARY KEY, p_id int NOT NULL)
21052105
CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id)
21062106
CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1)
21072107
CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL,
21082108
FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd)
21092109
) PARTITION BY list (id);
21102110
SET search_path TO fkpart12;
21112111

2112+
ALTER TABLE fk_p_1_2 DROP COLUMN x, DROP COLUMN y;
2113+
ALTER TABLE fk_p_1 ATTACH PARTITION fk_p_1_2 FOR VALUES IN (2);
2114+
ALTER TABLE fk_r_1 DROP COLUMN x;
2115+
21122116
INSERT INTO fk_p VALUES (1, 1);
21132117

21142118
ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);
@@ -2124,7 +2128,7 @@ ALTER TABLE fk_r DETACH PARTITION fk_r_2;
21242128

21252129
\d fk_r_2
21262130

2127-
INSERT INTO fk_r_1 VALUES (2, 1, 2); -- should fail
2131+
INSERT INTO fk_r_1 (id, p_id, p_jd) VALUES (2, 1, 2); -- should fail
21282132
DELETE FROM fk_p; -- should fail
21292133

21302134
ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);

0 commit comments

Comments
 (0)