diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 8867da6c69307..646578a460ea2 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -623,6 +623,15 @@ WITH ( MODULUS numeric_literal, REM If IF EXISTS is specified and the constraint does not exist, no error is thrown. In this case a notice is issued instead. + + For foreign key constraints, this acquires ACCESS + EXCLUSIVE lock on the table owning the constraint (as with + other ALTER TABLE forms), but only SHARE + ROW EXCLUSIVE lock on the referenced table. This allows + concurrent reads on the referenced table while the constraint is being + dropped. Other constraint types acquire ACCESS + EXCLUSIVE lock on the table being altered. + diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index fc89352b6617b..4bef2cfce8483 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -14165,7 +14165,8 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha * and check that that's not in use, just as we've already done for the * constrained table (else we might, eg, be dropping a trigger that has * unfired events). But we can/must skip that in the self-referential - * case. + * case. Use ShareRowExclusive to allow concurrent reads on the referenced + * table. */ if (con->contype == CONSTRAINT_FOREIGN && con->confrelid != RelationGetRelid(rel)) @@ -14173,7 +14174,7 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha Relation frel; /* Must match lock taken by RemoveTriggerById: */ - frel = table_open(con->confrelid, AccessExclusiveLock); + frel = table_open(con->confrelid, ShareRowExclusiveLock); CheckAlterTableIsSafe(frel); table_close(frel, NoLock); } @@ -15474,15 +15475,26 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) /* * When rebuilding another table's constraint that references the * table we're modifying, we might not yet have any lock on the other - * table, so get one now. We'll need AccessExclusiveLock for the DROP - * CONSTRAINT step, so there's no value in asking for anything weaker. + * table, so get one now. For foreign key constraints, use + * ShareRowExclusiveLock to allow readers on the referenced table + * during FK rebuild. For other constraints, use AccessExclusiveLock + * to avoid invalidating query plans that may rely on them. */ if (relid != tab->relid) - LockRelationOid(relid, AccessExclusiveLock); + { + LOCKMODE otherRelLockmode; + + if (con->contype == CONSTRAINT_FOREIGN && OidIsValid(confrelid)) + otherRelLockmode = ShareRowExclusiveLock; + else + otherRelLockmode = AccessExclusiveLock; + + LockRelationOid(relid, otherRelLockmode); + } ATPostAlterTypeParse(oldId, relid, confrelid, - (char *) lfirst(def_item), - wqueue, lockmode, tab->rewrite); + (char *) lfirst(def_item), + wqueue, lockmode, tab->rewrite); } forboth(oid_item, tab->changedIndexOids, def_item, tab->changedIndexDefs) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 579ac8d76ae73..5fe318c695ee9 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -1315,11 +1315,63 @@ RemoveTriggerById(Oid trigOid) elog(ERROR, "could not find tuple for trigger %u", trigOid); /* - * Open and exclusive-lock the relation the trigger belongs to. + * Open and lock the relation the trigger belongs to. + * + * For internal RI triggers on the referenced table (action triggers), we + * can use ShareRowExclusiveLock to allow concurrent reads while blocking + * writers. This is safe because these triggers don't affect query plans + * on the referenced table. + * + * For all other triggers (user triggers, RI check triggers on the FK + * table), we must use AccessExclusiveLock because dropping them could + * invalidate query plans that were optimized assuming the constraint or + * trigger behavior. */ - relid = ((Form_pg_trigger) GETSTRUCT(tup))->tgrelid; + { + Form_pg_trigger tgform = (Form_pg_trigger) GETSTRUCT(tup); + LOCKMODE lockmode; + + relid = tgform->tgrelid; + + /* + * Use weaker lock only for internal constraint triggers where the + * trigger's table is the referenced (not constrained) table. We + * identify these as internal triggers with a valid tgconstraint where + * tgrelid != the constraint's conrelid. + */ + if (tgform->tgisinternal && OidIsValid(tgform->tgconstraint)) + { + HeapTuple contup; + Form_pg_constraint conform; + + contup = SearchSysCache1(CONSTROID, + ObjectIdGetDatum(tgform->tgconstraint)); + if (HeapTupleIsValid(contup)) + { + conform = (Form_pg_constraint) GETSTRUCT(contup); - rel = table_open(relid, AccessExclusiveLock); + /* + * RI action triggers are on the referenced table (confrelid), + * not on the table owning the constraint (conrelid). Use + * weaker lock only for those; check triggers on the FK table + * need AccessExclusive to avoid breaking query plans. + */ + if (conform->contype == CONSTRAINT_FOREIGN && + tgform->tgrelid == conform->confrelid && + conform->confrelid != conform->conrelid) + lockmode = ShareRowExclusiveLock; + else + lockmode = AccessExclusiveLock; + ReleaseSysCache(contup); + } + else + lockmode = AccessExclusiveLock; + } + else + lockmode = AccessExclusiveLock; + + rel = table_open(relid, lockmode); + } if (rel->rd_rel->relkind != RELKIND_RELATION && rel->rd_rel->relkind != RELKIND_VIEW && diff --git a/src/test/isolation/expected/detach-partition-concurrently-4.out b/src/test/isolation/expected/detach-partition-concurrently-4.out index 79b29ae4c1033..09c34512f0c18 100644 --- a/src/test/isolation/expected/detach-partition-concurrently-4.out +++ b/src/test/isolation/expected/detach-partition-concurrently-4.out @@ -360,6 +360,30 @@ step s3commit: commit; step s1c: commit; step s2detach: <... completed> +starting permutation: s2snitch s1b s1write s2detach s1cancel s1c +step s2snitch: insert into d4_pid select pg_backend_pid(); +step s1b: begin; +step s1write: update d4_fk set a = a where a = 2; +step s2detach: alter table d4_primary detach partition d4_primary1 concurrently; +step s1cancel: select pg_cancel_backend(pid) from d4_pid; +step s2detach: <... completed> +ERROR: canceling statement due to user request +step s1cancel: <... completed> +pg_cancel_backend +----------------- +t +(1 row) + +step s1c: commit; + +starting permutation: s2snitch s1b s1write s2detach s1c +step s2snitch: insert into d4_pid select pg_backend_pid(); +step s1b: begin; +step s1write: update d4_fk set a = a where a = 2; +step s2detach: alter table d4_primary detach partition d4_primary1 concurrently; +step s1c: commit; +step s2detach: <... completed> + starting permutation: s2snitch s1brr s1s s2detach s1cancel s1noop s3vacfreeze s1s s1insert s1c step s2snitch: insert into d4_pid select pg_backend_pid(); step s1brr: begin isolation level repeatable read; diff --git a/src/test/isolation/expected/fk-drop-constraint-concurrency.out b/src/test/isolation/expected/fk-drop-constraint-concurrency.out new file mode 100644 index 0000000000000..385280861a2ce --- /dev/null +++ b/src/test/isolation/expected/fk-drop-constraint-concurrency.out @@ -0,0 +1,98 @@ +Parsed test spec with 2 sessions + +starting permutation: s1b s1s s2drop s1c +step s1b: begin; +step s1s: select * from pk; +id +-- + 1 +(1 row) + +step s2drop: alter table fk drop constraint fk_ref_fkey; +step s1c: commit; + +starting permutation: s1b s1declare_fk s1fetch_fk s2drop s1c +step s1b: begin; +step s1declare_fk: declare cfk cursor for select * from fk; +step s1fetch_fk: fetch 1 from cfk; +id|ref +--+--- + 1| 1 +(1 row) + +step s2drop: alter table fk drop constraint fk_ref_fkey; +step s1c: commit; +step s2drop: <... completed> + +starting permutation: s1b s1wfk s2drop s1c +step s1b: begin; +step s1wfk: update fk set ref = ref where id = 1; +step s2drop: alter table fk drop constraint fk_ref_fkey; +step s1c: commit; +step s2drop: <... completed> + +starting permutation: s1b s1wpk s2drop s1c +step s1b: begin; +step s1wpk: update pk set id = id where id = 1; +step s2drop: alter table fk drop constraint fk_ref_fkey; +step s1c: commit; +step s2drop: <... completed> + +starting permutation: s1prep s1b s1exec s2drop s1exec s1c +step s1prep: prepare q as select count(*) from pk; +step s1b: begin; +step s1exec: execute q; +count +----- + 1 +(1 row) + +step s2drop: alter table fk drop constraint fk_ref_fkey; +step s1exec: execute q; +count +----- + 1 +(1 row) + +step s1c: commit; + +starting permutation: s1b s1s s2drop_table s1c +step s1b: begin; +step s1s: select * from pk; +id +-- + 1 +(1 row) + +step s2drop_table: drop table fk; +step s1c: commit; + +starting permutation: s1b s1s_self s2drop_self s1c +step s1b: begin; +step s1s_self: select * from fkself; +id|parent +--+------ + 1| +(1 row) + +step s2drop_self: alter table fkself drop constraint fkself_parent_fkey; +step s1c: commit; +step s2drop_self: <... completed> + +starting permutation: s1b s1w_self s2drop_self s1c +step s1b: begin; +step s1w_self: update fkself set id = id where id = 1; +step s2drop_self: alter table fkself drop constraint fkself_parent_fkey; +step s1c: commit; +step s2drop_self: <... completed> + +starting permutation: s1b s1s s2alter_type s1c +step s1b: begin; +step s1s: select * from pk; +id +-- + 1 +(1 row) + +step s2alter_type: alter table fk_alttype alter column ref type bigint; +step s1c: commit; diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 5afae33d37036..987a3c0a1ae95 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -120,3 +120,4 @@ test: serializable-parallel-2 test: serializable-parallel-3 test: matview-write-skew test: lock-nowait +test: fk-drop-constraint-concurrency diff --git a/src/test/isolation/specs/detach-partition-concurrently-4.spec b/src/test/isolation/specs/detach-partition-concurrently-4.spec index 829c82248f139..dcd4791a2ffac 100644 --- a/src/test/isolation/specs/detach-partition-concurrently-4.spec +++ b/src/test/isolation/specs/detach-partition-concurrently-4.spec @@ -27,6 +27,7 @@ session s1 step s1b { begin; } step s1brr { begin isolation level repeatable read; } step s1s { select * from d4_primary; } +step s1write { update d4_fk set a = a where a = 2; } step s1cancel { select pg_cancel_backend(pid) from d4_pid; } step s1noop { } step s1insert { insert into d4_fk values (1); } @@ -77,6 +78,10 @@ permutation s2snitch s1b s1s s2detach s3insert s1c permutation s2snitch s1b s1s s2detach s3brr s3insert s3commit s1cancel(s2detach) s1c permutation s2snitch s1b s1s s2detach s3brr s3insert s3commit s1c +# Explicit coverage that a writer on the referencing table blocks detach +permutation s2snitch s1b s1write s2detach s1cancel(s2detach) s1c +permutation s2snitch s1b s1write s2detach s1c + # Try one where we VACUUM FREEZE pg_inherits (to verify that xmin change is # handled correctly). permutation s2snitch s1brr s1s s2detach s1cancel(s2detach) s1noop s3vacfreeze s1s s1insert s1c diff --git a/src/test/isolation/specs/fk-drop-constraint-concurrency.spec b/src/test/isolation/specs/fk-drop-constraint-concurrency.spec new file mode 100644 index 0000000000000..a9dfe7bd24e04 --- /dev/null +++ b/src/test/isolation/specs/fk-drop-constraint-concurrency.spec @@ -0,0 +1,74 @@ +# Regular-table analogue of detach-partition-concurrently behavior for FK drop. +# +# This spec exercises reduced relation-level locking (ShareRowExclusive) for +# FK/trigger removal: +# 1) SELECTs on the referenced table (pk) proceed while DROP CONSTRAINT runs. +# 2) Writers (UPDATE) on the referencing (fk) table block the FK drop. +# 3) Writers (UPDATE) on the referenced (pk) table block the FK drop. +# 4) Prepared SELECTs (plan + relcache) continue to work across FK drop. +# 5) DROP TABLE fktable doesn't block SELECTs on the referenced table. +# 6) Self-referential FK drops don't block SELECTs on the table. +# 7) ALTER COLUMN TYPE with FK rebuild doesn't block SELECTs on referenced table. + +setup { + drop table if exists pk, fk, fkself, fk_alttype; + create table pk(id int primary key); + create table fk(id int primary key, ref int references pk(id)); + create table fkself(id int primary key, parent int references fkself(id)); + create table fk_alttype(id int primary key, ref int references pk(id)); + insert into pk values (1); + insert into fk values (1, 1); + insert into fkself values (1, NULL); + insert into fk_alttype values (1, 1); +} + +teardown { + drop table if exists fk, fkself, fk_alttype, pk; +} + +session s1 +step s1b { begin; } +step s1s { select * from pk; } +step s1declare_fk { declare cfk cursor for select * from fk; } +step s1fetch_fk { fetch 1 from cfk; } +step s1s_self { select * from fkself; } +step s1wfk { update fk set ref = ref where id = 1; } +step s1wpk { update pk set id = id where id = 1; } +step s1w_self { update fkself set id = id where id = 1; } +step s1c { commit; } +step s1prep { prepare q as select count(*) from pk; } +step s1exec { execute q; } + +session s2 +step s2drop { alter table fk drop constraint fk_ref_fkey; } +step s2drop_table { drop table fk; } +step s2drop_self { alter table fkself drop constraint fkself_parent_fkey; } +step s2alter_type { alter table fk_alttype alter column ref type bigint; } + +# (1) Readers do not block FK drop +permutation s1b s1s s2drop s1c + +# (1a) SELECT on referencing table blocks FK drop (ALTER TABLE waits) +permutation s1b s1declare_fk s1fetch_fk s2drop s1c + +# (2) Writer on referencing table blocks FK drop +permutation s1b s1wfk s2drop s1c + +# (3) Writer on referenced table blocks FK drop +permutation s1b s1wpk s2drop s1c + +# (4) Prepared plan remains usable across FK drop; relcache invalidation ensures correctness +permutation s1prep s1b s1exec s2drop s1exec s1c + +# (5) DROP TABLE fktable does not block SELECTs on referenced table +permutation s1b s1s s2drop_table s1c + +# (6) Self-referential FK drop: ALTER TABLE takes AccessExclusive on the table +# being altered, so even SELECTs block (different from non-self-ref case) +permutation s1b s1s_self s2drop_self s1c + +# (7) Writer on self-referential FK table blocks drop of self-referential FK +permutation s1b s1w_self s2drop_self s1c + +# (8) ALTER COLUMN TYPE rebuilding FK: SELECTs on referenced table proceed +permutation s1b s1s s2alter_type s1c