diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml index 2ca423c..ccff611 100644 --- a/doc/src/sgml/mvcc.sgml +++ b/doc/src/sgml/mvcc.sgml @@ -865,7 +865,7 @@ ERROR: could not serialize access due to read/write dependencies among transact Acquired by VACUUM (without ), ANALYZE, CREATE INDEX CONCURRENTLY, and - some forms of ALTER TABLE. + ALTER TABLE VALIDATE. @@ -906,8 +906,8 @@ ERROR: could not serialize access due to read/write dependencies among transact - This lock mode is not automatically acquired by any - PostgreSQL command. + Acquired by ALTER TABLE for subcommand types that + affect write operations and by CREATE TRIGGER. @@ -951,7 +951,7 @@ ERROR: could not serialize access due to read/write dependencies among transact - Acquired by the ALTER TABLE, DROP TABLE, + Acquired by the ALTER TABLE for rewriting, DROP TABLE, TRUNCATE, REINDEX, CLUSTER, and VACUUM FULL commands. diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 89649a2..ae8df93 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -84,7 +84,10 @@ ALTER TABLE [ IF EXISTS ] name ALTER TABLE changes the definition of an existing table. - There are several subforms: + There are several subforms described below. Note that the lock level required + differs for each subform. An ACCESS EXCLUSIVE lock is held + unless explicitly noted. When multiple subcommands are listed, the lock + held will be the strictest one required from any subcommand. @@ -153,6 +156,9 @@ ALTER TABLE [ IF EXISTS ] name or UPDATE commands; they do not cause rows already in the table to change. + + This form requires only an SHARE ROW EXCLUSIVE lock. + @@ -181,6 +187,9 @@ ALTER TABLE [ IF EXISTS ] name PostgreSQL query planner, refer to . + + This form requires only an SHARE UPDATE EXCLUSIVE lock. + @@ -213,6 +222,9 @@ ALTER TABLE [ IF EXISTS ] name of statistics by the PostgreSQL query planner, refer to . + + This form requires only an SHARE UPDATE EXCLUSIVE lock. + @@ -264,6 +276,12 @@ ALTER TABLE [ IF EXISTS ] name the table, until it is validated by using the VALIDATE CONSTRAINT option. + + This form requires only a SHARE ROW EXCLUSIVE lock + on the table being altered. If the constraint is a foreign key then + a SHARE UPDATE EXCLUSIVE lock is also required on + the table referenced by the constraint. + @@ -325,6 +343,9 @@ ALTER TABLE [ IF EXISTS ] name This form alters the attributes of a constraint that was previously created. Currently only foreign key constraints may be altered. + + This form requires only an SHARE ROW EXCLUSIVE lock. + @@ -338,11 +359,17 @@ ALTER TABLE [ IF EXISTS ] name Nothing happens if the constraint is already marked valid. - Validation can be a long process on larger tables and currently requires - an ACCESS EXCLUSIVE lock. The value of separating + Validation can be a long process on larger tables. The value of separating validation from initial creation is that you can defer validation to less busy times, or can be used to give additional time to correct pre-existing - errors while preventing new errors. + errors while preventing new errors. Note also that validation on its own + does not prevent normal write commands against the table while it runs. + + + This form requires only an SHARE UPDATE EXCLUSIVE lock + on the table being altered. If the constraint is a foreign key then + a ROW SHARE lock is also required on + the table referenced by the constraint. @@ -383,6 +410,9 @@ ALTER TABLE [ IF EXISTS ] name mode, and triggers configured as ENABLE ALWAYS will fire regardless of the current replication mode. + + This form requires only an SHARE ROW EXCLUSIVE lock. + @@ -408,6 +438,9 @@ ALTER TABLE [ IF EXISTS ] name operations. It does not actually re-cluster the table. + + This form requires only an SHARE UPDATE EXCLUSIVE lock. + @@ -420,6 +453,9 @@ ALTER TABLE [ IF EXISTS ] name index specification from the table. This affects future cluster operations that don't specify an index. + + This form requires only an SHARE UPDATE EXCLUSIVE lock. + @@ -467,6 +503,9 @@ ALTER TABLE [ IF EXISTS ] name FULL, or one of the forms of ALTER TABLE that forces a table rewrite. + + This form requires only an SHARE ROW EXCLUSIVE lock. + @@ -489,6 +528,9 @@ ALTER TABLE [ IF EXISTS ] name defaults. As with SET, a table rewrite might be needed to update the table entirely. + + This form requires only an SHARE ROW EXCLUSIVE lock. + @@ -517,6 +559,9 @@ ALTER TABLE [ IF EXISTS ] name FOREIGN KEY constraints are not considered, but this might change in the future. + + This form requires only an SHARE UPDATE EXCLUSIVE lock. + @@ -529,6 +574,9 @@ ALTER TABLE [ IF EXISTS ] name Queries against the parent table will no longer include records drawn from the target table. + + This form requires only an SHARE UPDATE EXCLUSIVE lock. + @@ -544,6 +592,9 @@ ALTER TABLE [ IF EXISTS ] name that CREATE TABLE OF would permit an equivalent table definition. + + This form requires only an SHARE UPDATE EXCLUSIVE lock. + @@ -553,6 +604,9 @@ ALTER TABLE [ IF EXISTS ] name This form dissociates a typed table from its type. + + This form requires only an SHARE UPDATE EXCLUSIVE lock. + diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 466d757..a6baaad 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -2664,8 +2664,7 @@ AlterTableLookupRelation(AlterTableStmt *stmt, LOCKMODE lockmode) * The caller must lock the relation, with an appropriate lock level * for the subcommands requested. Any subcommand that needs to rewrite * tuples in the table forces the whole command to be executed with - * AccessExclusiveLock (actually, that is currently required always, but - * we hope to relax it at some point). We pass the lock level down + * AccessExclusiveLock. We pass the lock level down * so that we can apply it recursively to inherited tables. Note that the * lock level we want as we recurse might well be higher than required for * that specific subcommand. So we pass down the overall lock requirement, @@ -2732,30 +2731,8 @@ LOCKMODE AlterTableGetLockLevel(List *cmds) { /* - * Late in 9.1 dev cycle a number of issues were uncovered with access to - * catalog relations, leading to the decision to re-enforce all DDL at - * AccessExclusiveLock level by default. - * - * The issues are that there is a pervasive assumption in the code that - * the catalogs will not be read unless an AccessExclusiveLock is held. If - * that rule is relaxed, we must protect against a number of potential - * effects - infrequent, but proven possible with test cases where - * multiple DDL operations occur in a stream against frequently accessed - * tables. - * - * 1. Catalog tables were read using SnapshotNow, which has a race bug that - * allows a scan to return no valid rows even when one is present in the - * case of a commit of a concurrent update of the catalog table. - * SnapshotNow also ignores transactions in progress, so takes the latest - * committed version without waiting for the latest changes. - * - * 2. Relcache needs to be internally consistent, so unless we lock the - * definition during reads we have no way to guarantee that. - * - * 3. Catcache access isn't coordinated at all so refreshes can occur at - * any time. + * This only works if we read catalog tables using MVCC snapshots. */ -#ifdef REDUCED_ALTER_TABLE_LOCK_LEVELS ListCell *lcmd; LOCKMODE lockmode = ShareUpdateExclusiveLock; @@ -2789,6 +2766,7 @@ AlterTableGetLockLevel(List *cmds) case AT_SetTableSpace: /* must rewrite heap */ case AT_DropNotNull: /* may change some SQL plans */ case AT_SetNotNull: + case AT_SetStorage: /* may add toast tables, see ATRewriteCatalogs() */ case AT_GenericOptions: case AT_AlterColumnGenericOptions: cmd_lockmode = AccessExclusiveLock; @@ -2809,6 +2787,7 @@ AlterTableGetLockLevel(List *cmds) case AT_DisableTrig: case AT_DisableTrigAll: case AT_DisableTrigUser: + case AT_AlterConstraint: case AT_AddIndex: /* from ADD CONSTRAINT */ case AT_AddIndexConstraint: case AT_ReplicaIdentity: @@ -2891,8 +2870,6 @@ AlterTableGetLockLevel(List *cmds) case AT_ReplaceRelOptions: case AT_SetOptions: case AT_ResetOptions: - case AT_SetStorage: - case AT_AlterConstraint: case AT_ValidateConstraint: cmd_lockmode = ShareUpdateExclusiveLock; break; @@ -2909,9 +2886,6 @@ AlterTableGetLockLevel(List *cmds) if (cmd_lockmode > lockmode) lockmode = cmd_lockmode; } -#else - LOCKMODE lockmode = AccessExclusiveLock; -#endif return lockmode; } @@ -3239,6 +3213,13 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode) } } + /* + * If we think we might need to add/re-add toast tables then + * we currently need to hold an AccessExclusiveLock. + */ + if (lockmode < AccessExclusiveLock) + return; + /* Check to see if a toast table must be added. */ foreach(ltab, *wqueue) { @@ -5845,7 +5826,7 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel, * table; trying to start with a lesser lock will just create a risk of * deadlock.) */ - pkrel = heap_openrv(fkconstraint->pktable, AccessExclusiveLock); + pkrel = heap_openrv(fkconstraint->pktable, ShareUpdateExclusiveLock); /* * 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 86449a6..b3b1f7a 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -147,7 +147,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, ObjectAddress myself, referenced; - rel = heap_openrv(stmt->relation, AccessExclusiveLock); + rel = heap_openrv(stmt->relation, ShareUpdateExclusiveLock); /* * Triggers must be on tables or views, and there are additional @@ -482,8 +482,8 @@ 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 AccessExclusiveLock on the - * relation, so the trigger set won't be changing underneath us. + * NOTE that this is cool only because we have a sufficient lock on the + * relation to ensure the trigger set won't be changing underneath us. */ if (!isInternal) { @@ -1059,7 +1059,7 @@ RemoveTriggerById(Oid trigOid) */ relid = ((Form_pg_trigger) GETSTRUCT(tup))->tgrelid; - rel = heap_open(relid, AccessExclusiveLock); + rel = heap_open(relid, ShareUpdateExclusiveLock); if (rel->rd_rel->relkind != RELKIND_RELATION && rel->rd_rel->relkind != RELKIND_VIEW) @@ -1225,8 +1225,8 @@ renametrig(RenameStmt *stmt) * on tgrelid/tgname would complain anyway) and to ensure a trigger does * exist with oldname. * - * NOTE that this is cool only because we have AccessExclusiveLock on the - * relation, so the trigger set won't be changing underneath us. + * NOTE that this is cool only because we have a sufficient lock on the + * relation to ensure that the trigger set won't be changing underneath us. */ tgrel = heap_open(TriggerRelationId, RowExclusiveLock); diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 0f0c638..316d789 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1840,72 +1840,75 @@ and relnamespace != (select oid from pg_namespace where nspname = 'pg_catalog') and c.relname != 'my_locks' group by c.relname; create table alterlock (f1 int primary key, f2 text); +insert into alterlock values (1, 'foo'); +create table alterlock2 (f3 int primary key, f1 int); +insert into alterlock2 values (1, 1); begin; alter table alterlock alter column f2 set statistics 150; select * from my_locks order by 1; - relname | max_lockmode ------------+--------------------- - alterlock | AccessExclusiveLock + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock (1 row) rollback; begin; alter table alterlock cluster on alterlock_pkey; select * from my_locks order by 1; - relname | max_lockmode -----------------+--------------------- - alterlock | AccessExclusiveLock - alterlock_pkey | AccessExclusiveLock + relname | max_lockmode +----------------+-------------------------- + alterlock | ShareUpdateExclusiveLock + alterlock_pkey | ShareUpdateExclusiveLock (2 rows) commit; begin; alter table alterlock set without cluster; select * from my_locks order by 1; - relname | max_lockmode ------------+--------------------- - alterlock | AccessExclusiveLock + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock (1 row) commit; begin; alter table alterlock set (fillfactor = 100); select * from my_locks order by 1; - relname | max_lockmode ------------+--------------------- - alterlock | AccessExclusiveLock - pg_toast | AccessExclusiveLock + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock + pg_toast | ShareUpdateExclusiveLock (2 rows) commit; begin; alter table alterlock reset (fillfactor); select * from my_locks order by 1; - relname | max_lockmode ------------+--------------------- - alterlock | AccessExclusiveLock - pg_toast | AccessExclusiveLock + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock + pg_toast | ShareUpdateExclusiveLock (2 rows) commit; begin; alter table alterlock set (toast.autovacuum_enabled = off); select * from my_locks order by 1; - relname | max_lockmode ------------+--------------------- - alterlock | AccessExclusiveLock - pg_toast | AccessExclusiveLock + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock + pg_toast | ShareUpdateExclusiveLock (2 rows) commit; begin; alter table alterlock set (autovacuum_enabled = off); select * from my_locks order by 1; - relname | max_lockmode ------------+--------------------- - alterlock | AccessExclusiveLock - pg_toast | AccessExclusiveLock + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock + pg_toast | ShareUpdateExclusiveLock (2 rows) commit; begin; alter table alterlock alter column f2 set (n_distinct = 1); select * from my_locks order by 1; - relname | max_lockmode ------------+--------------------- - alterlock | AccessExclusiveLock + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock (1 row) rollback; @@ -1919,13 +1922,67 @@ select * from my_locks order by 1; rollback; begin; alter table alterlock alter column f2 set default 'x'; select * from my_locks order by 1; - relname | max_lockmode ------------+--------------------- - alterlock | AccessExclusiveLock + relname | max_lockmode +-----------+----------------------- + alterlock | ShareRowExclusiveLock +(1 row) + +rollback; +begin; +create trigger ttdummy + before delete or update on alterlock + for each row + execute procedure + ttdummy (1, 1); +select * from my_locks order by 1; + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock (1 row) rollback; +begin; +select * from my_locks order by 1; + relname | max_lockmode +---------+-------------- +(0 rows) + +alter table alterlock2 add foreign key (f1) references alterlock (f1); +select * from my_locks order by 1; + relname | max_lockmode +-----------------+-------------------------- + alterlock | ShareUpdateExclusiveLock + alterlock2 | ShareRowExclusiveLock + alterlock2_pkey | AccessShareLock + alterlock_pkey | AccessShareLock +(4 rows) + +rollback; +begin; +alter table alterlock2 +add constraint alterlock2nv foreign key (f1) references alterlock (f1) NOT VALID; +select * from my_locks order by 1; + relname | max_lockmode +------------+-------------------------- + alterlock | ShareUpdateExclusiveLock + alterlock2 | ShareRowExclusiveLock +(2 rows) + +commit; +begin; +alter table alterlock2 validate constraint alterlock2nv; +select * from my_locks order by 1; + relname | max_lockmode +-----------------+-------------------------- + alterlock | RowShareLock + alterlock2 | ShareUpdateExclusiveLock + alterlock2_pkey | AccessShareLock + alterlock_pkey | AccessShareLock +(4 rows) + +rollback; -- cleanup +drop table alterlock2; drop table alterlock; drop view my_locks; drop type lockmodes; diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 87973c1..a2ad863 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1283,6 +1283,9 @@ and c.relname != 'my_locks' group by c.relname; create table alterlock (f1 int primary key, f2 text); +insert into alterlock values (1, 'foo'); +create table alterlock2 (f3 int primary key, f1 int); +insert into alterlock2 values (1, 1); begin; alter table alterlock alter column f2 set statistics 150; select * from my_locks order by 1; @@ -1324,7 +1327,33 @@ begin; alter table alterlock alter column f2 set default 'x'; select * from my_locks order by 1; rollback; +begin; +create trigger ttdummy + before delete or update on alterlock + for each row + execute procedure + ttdummy (1, 1); +select * from my_locks order by 1; +rollback; + +begin; +select * from my_locks order by 1; +alter table alterlock2 add foreign key (f1) references alterlock (f1); +select * from my_locks order by 1; +rollback; + +begin; +alter table alterlock2 +add constraint alterlock2nv foreign key (f1) references alterlock (f1) NOT VALID; +select * from my_locks order by 1; +commit; +begin; +alter table alterlock2 validate constraint alterlock2nv; +select * from my_locks order by 1; +rollback; + -- cleanup +drop table alterlock2; drop table alterlock; drop view my_locks; drop type lockmodes;