diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml index 1d337b8055..1ec07dd9ee 100644 --- a/doc/src/sgml/mvcc.sgml +++ b/doc/src/sgml/mvcc.sgml @@ -878,9 +878,8 @@ ERROR: could not serialize access due to read/write dependencies among transact - Acquired by CREATE TRIGGER, - CREATE RULE (except for ON SELECT - rules) and some forms of ALTER TABLE. + This lock mode is not automatically acquired by any + PostgreSQL command. @@ -925,10 +924,10 @@ ERROR: could not serialize access due to read/write dependencies among transact - Acquired by the DROP TABLE, + Acquired by the ALTER TABLE, DROP TABLE, TRUNCATE, REINDEX, CLUSTER, and VACUUM FULL - commands, and some forms of ALTER TABLE. + commands. This is also the default lock mode for LOCK TABLE statements that do not specify a mode explicitly. diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 3bc350a113..c1af12a5a3 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -3425,14 +3425,12 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode) Relation refrel; if (rel == NULL) + { /* Long since locked, no need for another */ rel = heap_open(tab->relid, NoLock); + } - /* - * We're adding a trigger to both tables, so the lock level - * here should sensibly reflect that. - */ - refrel = heap_open(con->refrelid, ShareRowExclusiveLock); + refrel = heap_open(con->refrelid, RowShareLock); validateForeignKeyConstraint(fkconstraint->conname, rel, refrel, con->refindid, @@ -5529,7 +5527,14 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel, Oid indexOid; Oid constrOid; - pkrel = heap_openrv(fkconstraint->pktable, lockmode); + /* + * Grab an exclusive lock on the pk table, so that someone doesn't delete + * rows out from under us. (Although a lesser lock would do for that + * purpose, we'll need exclusive lock anyway to add triggers to the pk + * table; trying to start with a lesser lock will just create a risk of + * deadlock.) + */ + pkrel = heap_openrv(fkconstraint->pktable, AccessExclusiveLock); /* * Validity checks (permission checks wait till we have the column diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 8072c77687..9cf8372e96 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -144,14 +144,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, ObjectAddress myself, referenced; - /* - * ShareRowExclusiveLock is sufficient to prevent concurrent write - * activity to the relation, and thus to lock out any operations that - * might want to fire triggers on the relation. If we had ON SELECT - * triggers we would need to take an AccessExclusiveLock to add one of - * those, just as we do with ON SELECT rules. - */ - rel = heap_openrv(stmt->relation, ShareRowExclusiveLock); + rel = heap_openrv(stmt->relation, AccessExclusiveLock); /* * Triggers must be on tables or views, and there are additional @@ -481,7 +474,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, * can skip this for internally generated triggers, since the name * modification above should be sufficient. * - * NOTE that this is cool only because we have ShareRowExclusiveLock on + * NOTE that this is cool only because we have AccessExclusiveLock on * the relation, so the trigger set won't be changing underneath us. */ if (!isInternal) @@ -1085,14 +1078,11 @@ RemoveTriggerById(Oid trigOid) elog(ERROR, "could not find tuple for trigger %u", trigOid); /* - * Open and lock the relation the trigger belongs to. As in - * CreateTrigger, this is sufficient to lock out all operations that could - * fire or add triggers; but it would need to be revisited if we had ON - * SELECT triggers. + * Open and exclusive-lock the relation the trigger belongs to. */ relid = ((Form_pg_trigger) GETSTRUCT(tup))->tgrelid; - rel = heap_open(relid, ShareRowExclusiveLock); + rel = heap_open(relid, AccessExclusiveLock); if (rel->rd_rel->relkind != RELKIND_RELATION && rel->rd_rel->relkind != RELKIND_VIEW) diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c index bb7d468d19..60b988175b 100644 --- a/src/backend/rewrite/rewriteDefine.c +++ b/src/backend/rewrite/rewriteDefine.c @@ -238,14 +238,12 @@ DefineQueryRewrite(char *rulename, /* * If we are installing an ON SELECT rule, we had better grab * AccessExclusiveLock to ensure no SELECTs are currently running on the - * event relation. For other types of rules, it is sufficient to grab - * ShareRowExclusiveLock to lock out insert/update/delete actions and to - * ensure that we lock out current CREATE RULE statements. + * event relation. For other types of rules, it would be sufficient to + * grab ShareRowExclusiveLock to lock out insert/update/delete actions and + * to ensure that we lock out current CREATE RULE statements; but because + * of race conditions in access to catalog entries, we can't do that yet. */ - if (event_type == CMD_SELECT) - event_relation = heap_open(event_relid, AccessExclusiveLock); - else - event_relation = heap_open(event_relid, ShareRowExclusiveLock); + event_relation = heap_open(event_relid, AccessExclusiveLock); /* * Verify relation is of a type that rules can sensibly be applied to.