open-dsi
/
postgres
mirror of https://github.com/postgres/postgres
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix some more bugs in foreign keys connecting partitioned tables

Browse Source 
* 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 53af9491a0.
  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 53af9491a0, 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
REL_15_STABLE
Álvaro Herrera 11 months ago
parent 0fe002d0c9
commit 1b216fceff
No known key found for this signature in database
GPG Key ID: 1C20ACB9D5C564AE
3 changed files with 28 additions and 30 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 39
      src/backend/commands/tablecmds.c
  2. 9
      src/test/regress/expected/foreign_key.out
  3. 10
      src/test/regress/sql/foreign_key.sql

39
src/backend/commands/tablecmds.c
Unescape View File

@ -9844,6 +9844,9 @@ addFkRecurseReferenced(Constraint *fkconstraint, Relation rel,
Oid deleteTriggerOid, Oid deleteTriggerOid,
updateTriggerOid; updateTriggerOid;
Assert(CheckRelationLockedByMe(pkrel, ShareRowExclusiveLock, true));
Assert(CheckRelationLockedByMe(rel, ShareRowExclusiveLock, true));
/* /*
* Create the action triggers that enforce the constraint. * Create the action triggers that enforce the constraint.
*/ */
@ -9870,6 +9873,7 @@ addFkRecurseReferenced(Constraint *fkconstraint, Relation rel,
Oid partIndexId; Oid partIndexId;
ObjectAddress address; ObjectAddress address;
/* XXX would it be better to acquire these locks beforehand? */
partRel = table_open(pd->oids[i], ShareRowExclusiveLock); partRel = table_open(pd->oids[i], ShareRowExclusiveLock);
/* /*
@ -9970,7 +9974,9 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel,
Oid insertTriggerOid, Oid insertTriggerOid,
updateTriggerOid; updateTriggerOid;
AssertArg(OidIsValid(parentConstr)); Assert(OidIsValid(parentConstr));
Assert(CheckRelationLockedByMe(rel, ShareRowExclusiveLock, true));
Assert(CheckRelationLockedByMe(pkrel, ShareRowExclusiveLock, true));
if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE) if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
ereport(ERROR, ereport(ERROR,
@ -10259,13 +10265,8 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
continue; continue;
} }
/* /* We need the same lock level that CreateTrigger will acquire */
* Because we're only expanding the key space at the referenced side, fkRel = table_open(constrForm->conrelid, ShareRowExclusiveLock);
* we don't need to prevent any operation in the referencing table, so
* AccessShareLock suffices (assumes that dropping the constraint
* acquires AccessExclusiveLock).
*/
fkRel = table_open(constrForm->conrelid, AccessShareLock);
indexOid = constrForm->conindid; indexOid = constrForm->conindid;
DeconstructFkConstraintRow(tuple, DeconstructFkConstraintRow(tuple,
@ -18822,8 +18823,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
foreach(cell, fks) foreach(cell, fks)
{ {
ForeignKeyCacheInfo *fk = lfirst(cell); ForeignKeyCacheInfo *fk = lfirst(cell);
HeapTuple contup, HeapTuple contup;
parentConTup;
Form_pg_constraint conform; Form_pg_constraint conform;
Oid insertTriggerOid, Oid insertTriggerOid,
updateTriggerOid; updateTriggerOid;
@ -18841,13 +18841,6 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
continue; continue;
} }
Assert(OidIsValid(conform->conparentid));
parentConTup = SearchSysCache1(CONSTROID,
ObjectIdGetDatum(conform->conparentid));
if (!HeapTupleIsValid(parentConTup))
elog(ERROR, "cache lookup failed for constraint %u",
conform->conparentid);
/* /*
* The constraint on this table must be marked no longer a child of * The constraint on this table must be marked no longer a child of
* the parent's constraint, as do its check triggers. * the parent's constraint, as do its check triggers.
@ -18888,7 +18881,6 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
Oid conffeqop[INDEX_MAX_KEYS]; Oid conffeqop[INDEX_MAX_KEYS];
int numfkdelsetcols; int numfkdelsetcols;
AttrNumber confdelsetcols[INDEX_MAX_KEYS]; AttrNumber confdelsetcols[INDEX_MAX_KEYS];
AttrMap *attmap;
Relation refdRel; Relation refdRel;
DeconstructFkConstraintRow(contup, DeconstructFkConstraintRow(contup,
@ -18921,19 +18913,19 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
fkconstraint->old_pktable_oid = InvalidOid; fkconstraint->old_pktable_oid = InvalidOid;
fkconstraint->location = -1; fkconstraint->location = -1;
attmap = build_attrmap_by_name(RelationGetDescr(partRel), /* set up colnames, used to generate the constraint name */
RelationGetDescr(rel));
for (int i = 0; i < numfks; i++) for (int i = 0; i < numfks; i++)
{ {
Form_pg_attribute att; Form_pg_attribute att;
att = TupleDescAttr(RelationGetDescr(partRel), att = TupleDescAttr(RelationGetDescr(partRel),
attmap->attnums[conkey[i] - 1] - 1); conkey[i] - 1);
fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs, fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs,
makeString(NameStr(att->attname))); makeString(NameStr(att->attname)));
} }
refdRel = table_open(fk->confrelid, AccessShareLock); refdRel = table_open(fk->confrelid, ShareRowExclusiveLock);
addFkRecurseReferenced(fkconstraint, partRel, addFkRecurseReferenced(fkconstraint, partRel,
refdRel, refdRel,
@ -18949,11 +18941,10 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
confdelsetcols, confdelsetcols,
true, true,
InvalidOid, InvalidOid); InvalidOid, InvalidOid);
table_close(refdRel, AccessShareLock); table_close(refdRel, NoLock); /* keep lock till end of xact */
} }
ReleaseSysCache(contup); ReleaseSysCache(contup);
ReleaseSysCache(parentConTup);
} }
list_free_deep(fks); list_free_deep(fks);
if (trigrel) if (trigrel)

9
src/test/regress/expected/foreign_key.out
Unescape View File

@ -2944,17 +2944,20 @@ CREATE SCHEMA fkpart12
CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id) CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id)
CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd) CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd)
CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1) CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1)
CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2) CREATE TABLE fk_p_1_2 (x int, y int, jd int NOT NULL, id int NOT NULL)
CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd) CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd)
CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1) CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1)
CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2) CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2)
CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) CREATE TABLE fk_r_1 ( p_jd int NOT NULL, x int, id int PRIMARY KEY, p_id int NOT NULL)
CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id) CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id)
CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1) CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1)
CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL, CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL,
FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd) FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd)
) PARTITION BY list (id); ) PARTITION BY list (id);
SET search_path TO fkpart12; SET search_path TO fkpart12;
ALTER TABLE fk_p_1_2 DROP COLUMN x, DROP COLUMN y;
ALTER TABLE fk_p_1 ATTACH PARTITION fk_p_1_2 FOR VALUES IN (2);
ALTER TABLE fk_r_1 DROP COLUMN x;
INSERT INTO fk_p VALUES (1, 1); INSERT INTO fk_p VALUES (1, 1);
ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);
ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2);
@ -2993,7 +2996,7 @@ Foreign-key constraints:
"fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd)
Number of partitions: 1 (Use \d+ to list them.) Number of partitions: 1 (Use \d+ to list them.)
INSERT INTO fk_r_1 VALUES (2, 1, 2); -- should fail INSERT INTO fk_r_1 (id, p_id, p_jd) VALUES (2, 1, 2); -- should fail
ERROR: insert or update on table "fk_r_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey" ERROR: insert or update on table "fk_r_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey"
DETAIL: Key (p_id, p_jd)=(1, 2) is not present in table "fk_p". DETAIL: Key (p_id, p_jd)=(1, 2) is not present in table "fk_p".
DELETE FROM fk_p; -- should fail DELETE FROM fk_p; -- should fail

10
src/test/regress/sql/foreign_key.sql
Unescape View File

@ -2097,11 +2097,11 @@ CREATE SCHEMA fkpart12
CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id) CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id)
CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd) CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd)
CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1) CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1)
CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2) CREATE TABLE fk_p_1_2 (x int, y int, jd int NOT NULL, id int NOT NULL)
CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd) CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd)
CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1) CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1)
CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2) CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2)
CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) CREATE TABLE fk_r_1 ( p_jd int NOT NULL, x int, id int PRIMARY KEY, p_id int NOT NULL)
CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id) CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id)
CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1) CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1)
CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL, CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL,
@ -2109,6 +2109,10 @@ CREATE SCHEMA fkpart12
) PARTITION BY list (id); ) PARTITION BY list (id);
SET search_path TO fkpart12; SET search_path TO fkpart12;
ALTER TABLE fk_p_1_2 DROP COLUMN x, DROP COLUMN y;
ALTER TABLE fk_p_1 ATTACH PARTITION fk_p_1_2 FOR VALUES IN (2);
ALTER TABLE fk_r_1 DROP COLUMN x;
INSERT INTO fk_p VALUES (1, 1); INSERT INTO fk_p VALUES (1, 1);
ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); 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;
\d fk_r_2 \d fk_r_2
INSERT INTO fk_r_1 VALUES (2, 1, 2); -- should fail INSERT INTO fk_r_1 (id, p_id, p_jd) VALUES (2, 1, 2); -- should fail
DELETE FROM fk_p; -- should fail DELETE FROM fk_p; -- should fail
ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);

Write Preview
Loading…
Cancel
Save