*** a/doc/src/sgml/mvcc.sgml --- b/doc/src/sgml/mvcc.sgml *************** *** 532,538 **** SELECT SUM(value) FROM mytab WHERE class = 2; most PostgreSQL commands automatically acquire locks of appropriate modes to ensure that referenced tables are not dropped or modified in incompatible ways while the ! command executes. (For example, ALTER TABLE cannot safely be executed concurrently with other operations on the same table, so it obtains an exclusive lock on the table to enforce that.) --- 532,538 ---- most PostgreSQL commands automatically acquire locks of appropriate modes to ensure that referenced tables are not dropped or modified in incompatible ways while the ! command executes. (For example, TRUNCATE cannot safely be executed concurrently with other operations on the same table, so it obtains an exclusive lock on the table to enforce that.) *************** *** 695,702 **** SELECT SUM(value) FROM mytab WHERE class = 2; ! This lock mode is not automatically acquired by any ! PostgreSQL command. --- 695,703 ---- ! Acquired by CREATE TRIGGER, ! CREATE RULE (except for ON SELECT ! rules) and in some cases ALTER TABLE. *************** *** 742,752 **** SELECT SUM(value) FROM mytab WHERE class = 2; ! Acquired by the ALTER TABLE, DROP ! TABLE, TRUNCATE, REINDEX, CLUSTER, and VACUUM FULL ! commands. This is also the default lock mode for LOCK ! TABLE statements that do not specify a mode explicitly. --- 743,754 ---- ! Acquired by the DROP TABLE, ! TRUNCATE, REINDEX, CLUSTER, and VACUUM FULL ! commands, as well as most variants of ALTER TABLE. ! This is also the default lock mode for LOCK TABLE ! statements that do not specify a mode explicitly. *** a/src/backend/commands/tablecmds.c --- b/src/backend/commands/tablecmds.c *************** *** 64,69 **** --- 64,70 ---- #include "rewrite/rewriteHandler.h" #include "storage/bufmgr.h" #include "storage/lmgr.h" + #include "storage/lock.h" #include "storage/smgr.h" #include "utils/acl.h" #include "utils/builtins.h" *************** *** 253,273 **** static void validateForeignKeyConstraint(Constraint *fkconstraint, Oid pkindOid, Oid constraintOid); static void createForeignKeyTriggers(Relation rel, Constraint *fkconstraint, Oid constraintOid, Oid indexOid); ! static void ATController(Relation rel, List *cmds, bool recurse); static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, ! bool recurse, bool recursing); ! static void ATRewriteCatalogs(List **wqueue); static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, ! AlterTableCmd *cmd); static void ATRewriteTables(List **wqueue); static void ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap); static AlteredTableInfo *ATGetQueueEntry(List **wqueue, Relation rel); static void ATSimplePermissions(Relation rel, bool allowView); static void ATSimplePermissionsRelationOrIndex(Relation rel); static void ATSimpleRecursion(List **wqueue, Relation rel, ! AlterTableCmd *cmd, bool recurse); static void ATOneLevelRecursion(List **wqueue, Relation rel, ! AlterTableCmd *cmd); static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, AlterTableCmd *cmd); static void ATExecAddColumn(AlteredTableInfo *tab, Relation rel, --- 254,274 ---- Oid pkindOid, Oid constraintOid); static void createForeignKeyTriggers(Relation rel, Constraint *fkconstraint, Oid constraintOid, Oid indexOid); ! static void ATController(Relation rel, List *cmds, bool recurse, LOCKMODE lockmode); static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, ! bool recurse, bool recursing, LOCKMODE lockmode); ! static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode); static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, ! AlterTableCmd *cmd, LOCKMODE lockmode); static void ATRewriteTables(List **wqueue); static void ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap); static AlteredTableInfo *ATGetQueueEntry(List **wqueue, Relation rel); static void ATSimplePermissions(Relation rel, bool allowView); static void ATSimplePermissionsRelationOrIndex(Relation rel); static void ATSimpleRecursion(List **wqueue, Relation rel, ! AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode); static void ATOneLevelRecursion(List **wqueue, Relation rel, ! AlterTableCmd *cmd, LOCKMODE lockmode); static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, AlterTableCmd *cmd); static void ATExecAddColumn(AlteredTableInfo *tab, Relation rel, *************** *** 293,308 **** static void ATExecDropColumn(List **wqueue, Relation rel, const char *colName, bool recurse, bool recursing, bool missing_ok); static void ATExecAddIndex(AlteredTableInfo *tab, Relation rel, ! IndexStmt *stmt, bool is_rebuild); static void ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ! Constraint *newConstraint, bool recurse); static void ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, Constraint *constr, ! bool recurse, bool recursing); static void ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel, ! Constraint *fkconstraint); static void ATExecDropConstraint(Relation rel, const char *constrName, DropBehavior behavior, bool recurse, bool recursing, --- 294,309 ---- bool recurse, bool recursing, bool missing_ok); static void ATExecAddIndex(AlteredTableInfo *tab, Relation rel, ! IndexStmt *stmt, bool is_rebuild, LOCKMODE lockmode); static void ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ! Constraint *newConstraint, bool recurse, LOCKMODE lockmode); static void ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, Constraint *constr, ! bool recurse, bool recursing, LOCKMODE lockmode); static void ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel, ! Constraint *fkconstraint, LOCKMODE lockmode); static void ATExecDropConstraint(Relation rel, const char *constrName, DropBehavior behavior, bool recurse, bool recursing, *************** *** 310,316 **** static void ATExecDropConstraint(Relation rel, const char *constrName, static void ATPrepAlterColumnType(List **wqueue, AlteredTableInfo *tab, Relation rel, bool recurse, bool recursing, ! AlterTableCmd *cmd); static void ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, const char *colName, TypeName *typeName); static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab); --- 311,317 ---- static void ATPrepAlterColumnType(List **wqueue, AlteredTableInfo *tab, Relation rel, bool recurse, bool recursing, ! AlterTableCmd *cmd, LOCKMODE lockmode); static void ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, const char *colName, TypeName *typeName); static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab); *************** *** 2237,2244 **** RenameRelationInternal(Oid myrelid, const char *newrelname, Oid namespaceId) * Disallow ALTER TABLE (and similar commands) when the current backend has * any open reference to the target table besides the one just acquired by * the calling command; this implies there's an open cursor or active plan. ! * We need this check because our AccessExclusiveLock doesn't protect us ! * against stomping on our own foot, only other people's feet! * * For ALTER TABLE, the only case known to cause serious trouble is ALTER * COLUMN TYPE, and some changes are obviously pretty benign, so this could --- 2238,2245 ---- * Disallow ALTER TABLE (and similar commands) when the current backend has * any open reference to the target table besides the one just acquired by * the calling command; this implies there's an open cursor or active plan. ! * We need this check because our lock doesn't protect us against stomping ! * on our own foot, only other people's feet! * * For ALTER TABLE, the only case known to cause serious trouble is ALTER * COLUMN TYPE, and some changes are obviously pretty benign, so this could *************** *** 2315,2325 **** CheckTableNotInUse(Relation rel, const char *stmt) * * Thanks to the magic of MVCC, an error anywhere along the way rolls back * the whole operation; we don't have to do anything special to clean up. */ void AlterTable(AlterTableStmt *stmt) { ! Relation rel = relation_openrv(stmt->relation, AccessExclusiveLock); CheckTableNotInUse(rel, "ALTER TABLE"); --- 2316,2342 ---- * * Thanks to the magic of MVCC, an error anywhere along the way rolls back * the whole operation; we don't have to do anything special to clean up. + * + * We lock the table as the first action, 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. If all subcommands do not require rewrite table + * then we may be able to use lower lock levels. 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 may well be higher than required for + * that specific subcommand. So we pass down the overall lock requirement, + * rather than reassess it at lower levels. */ void AlterTable(AlterTableStmt *stmt) { ! Relation rel; ! LOCKMODE lockmode = AlterTableGreatestLockLevel(stmt->cmds); ! ! /* ! * Acquire same level of lock as already acquired during parsing. ! */ ! rel = relation_openrv(stmt->relation, lockmode); CheckTableNotInUse(rel, "ALTER TABLE"); *************** *** 2362,2368 **** AlterTable(AlterTableStmt *stmt) elog(ERROR, "unrecognized object type: %d", (int) stmt->relkind); } ! ATController(rel, stmt->cmds, interpretInhOption(stmt->relation->inhOpt)); } /* --- 2379,2386 ---- elog(ERROR, "unrecognized object type: %d", (int) stmt->relkind); } ! ATController(rel, stmt->cmds, interpretInhOption(stmt->relation->inhOpt), ! lockmode); } /* *************** *** 2379,2391 **** AlterTable(AlterTableStmt *stmt) void AlterTableInternal(Oid relid, List *cmds, bool recurse) { ! Relation rel = relation_open(relid, AccessExclusiveLock); ! ATController(rel, cmds, recurse); } static void ! ATController(Relation rel, List *cmds, bool recurse) { List *wqueue = NIL; ListCell *lcmd; --- 2397,2559 ---- void AlterTableInternal(Oid relid, List *cmds, bool recurse) { ! Relation rel; ! LOCKMODE lockmode = AlterTableGreatestLockLevel(cmds); ! ! rel = relation_open(relid, lockmode); ! ! ATController(rel, cmds, recurse, lockmode); ! } ! ! /* ! * AlterTableGreatestLockLevel ! * ! * Sets the overall lock level required for the supplied list of subcommands. ! * Policy for doing this set according to needs of AlterTable(), see ! * comments there for overall explanation. ! * ! * Function is called before and after parsing, so it must give same ! * answer each time it is called. Some subcommands are transformed ! * into other subcommand types, so the transform must never be made to a ! * lower lock level than previously assigned. All transforms are noted below. ! * ! * Since this is called before we lock the table we cannot use table metadata ! * to influence the type of lock we acquire. ! */ ! LOCKMODE ! AlterTableGreatestLockLevel(List *cmds) ! { ! ListCell *lcmd; ! LOCKMODE lockmode = ShareUpdateExclusiveLock; /* default for compiler */ ! ! foreach(lcmd, cmds) ! { ! AlterTableCmd *cmd = (AlterTableCmd *) lfirst(lcmd); ! LOCKMODE cmd_lockmode = AccessExclusiveLock; /* default for compiler */ ! ! switch (cmd->subtype) ! { ! /* ! * Need AccessExclusiveLock for these subcommands because they ! * affect or potentially affect both read and write operations. ! * ! * New subcommand types should be added here by default. ! */ ! case AT_AddColumn: /* may rewrite heap, in some cases and visible to SELECT */ ! case AT_DropColumn: /* change visible to SELECT */ ! case AT_AddColumnToView: /* CREATE VIEW */ ! case AT_AlterColumnType: /* must rewrite heap */ ! case AT_DropConstraint: /* as DROP INDEX */ ! case AT_AddOids: ! case AT_DropOids: /* calls AT_DropColumn */ ! case AT_EnableAlwaysRule: /* as DROP INDEX */ ! case AT_EnableReplicaRule: /* as DROP INDEX */ ! case AT_EnableRule: /* as DROP INDEX */ ! case AT_DisableRule: /* as DROP INDEX */ ! case AT_ChangeOwner: /* change visible to SELECT */ ! case AT_SetTableSpace: /* must rewrite heap */ ! case AT_DropNotNull: /* may change some SQL plans */ ! case AT_SetNotNull: ! cmd_lockmode = AccessExclusiveLock; ! break; ! ! /* ! * These subcommands affect write operations only. ! */ ! case AT_ColumnDefault: ! case AT_ProcessedConstraint: /* becomes AT_AddConstraint */ ! case AT_AddConstraintRecurse: /* becomes AT_AddConstraint */ ! case AT_EnableTrig: ! case AT_EnableAlwaysTrig: ! case AT_EnableReplicaTrig: ! case AT_EnableTrigAll: ! case AT_EnableTrigUser: ! case AT_DisableTrig: ! case AT_DisableTrigAll: ! case AT_DisableTrigUser: ! case AT_AddIndex: /* from ADD CONSTRAINT */ ! cmd_lockmode = ShareRowExclusiveLock; ! break; ! ! case AT_AddConstraint: ! if (IsA(cmd->def, Constraint)) ! { ! Constraint *con = (Constraint *) cmd->def; ! ! switch (con->contype) ! { ! case CONSTR_EXCLUSION: ! case CONSTR_PRIMARY: ! case CONSTR_UNIQUE: ! /* ! * Cases essentially the same as CREATE INDEX. We ! * could reduce the lock strength to ShareLock if we ! * can work out how to allow concurrent catalog updates. ! */ ! cmd_lockmode = ShareRowExclusiveLock; ! break; ! case CONSTR_FOREIGN: ! /* ! * We add triggers to both tables when we add a ! * Foreign Key, so the lock level must be at least ! * as strong as CREATE TRIGGER. ! */ ! cmd_lockmode = ShareRowExclusiveLock; ! break; ! ! default: ! cmd_lockmode = ShareRowExclusiveLock; ! } ! } ! break; ! ! /* ! * These subcommands affect inheritance behaviour. Queries started before us ! * will continue to see the old inheritance behaviour, while queries started ! * after we commit will see new behaviour. No need to prevent reads or writes ! * to the subtable while we hook it up though. In both cases the parent table ! * is locked with AccessShareLock. ! */ ! case AT_AddInherit: ! case AT_DropInherit: ! cmd_lockmode = ShareUpdateExclusiveLock; ! break; ! ! /* ! * These subcommands affect general strategies for performance and maintenance, ! * though don't change the semantic results from normal data reads and writes. ! * Delaying an ALTER TABLE behind currently active writes only delays the point ! * where the new strategy begins to take effect, so there is no benefit in waiting. ! * In thise case the minimum restriction applies: we don't currently allow ! * concurrent catalog updates. ! */ ! case AT_SetStatistics: ! case AT_ClusterOn: ! case AT_DropCluster: ! case AT_SetRelOptions: ! case AT_ResetRelOptions: ! case AT_SetStorage: ! cmd_lockmode = ShareUpdateExclusiveLock; ! break; ! ! default: /* oops */ ! elog(ERROR, "unrecognized alter table type: %d", ! (int) cmd->subtype); ! break; ! } ! ! /* ! * Take the greatest lockmode from any subcommand ! */ ! if (cmd_lockmode > lockmode) ! lockmode = cmd_lockmode; ! } ! return lockmode; } static void ! ATController(Relation rel, List *cmds, bool recurse, LOCKMODE lockmode) { List *wqueue = NIL; ListCell *lcmd; *************** *** 2395,2408 **** ATController(Relation rel, List *cmds, bool recurse) { AlterTableCmd *cmd = (AlterTableCmd *) lfirst(lcmd); ! ATPrepCmd(&wqueue, rel, cmd, recurse, false); } /* Close the relation, but keep lock until commit */ relation_close(rel, NoLock); /* Phase 2: update system catalogs */ ! ATRewriteCatalogs(&wqueue); /* Phase 3: scan/rewrite tables as needed */ ATRewriteTables(&wqueue); --- 2563,2576 ---- { AlterTableCmd *cmd = (AlterTableCmd *) lfirst(lcmd); ! ATPrepCmd(&wqueue, rel, cmd, recurse, false, lockmode); } /* Close the relation, but keep lock until commit */ relation_close(rel, NoLock); /* Phase 2: update system catalogs */ ! ATRewriteCatalogs(&wqueue, lockmode); /* Phase 3: scan/rewrite tables as needed */ ATRewriteTables(&wqueue); *************** *** 2414,2425 **** ATController(Relation rel, List *cmds, bool recurse) * Traffic cop for ALTER TABLE Phase 1 operations, including simple * recursion and permission checks. * ! * Caller must have acquired AccessExclusiveLock on relation already. * This lock should be held until commit. */ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, ! bool recurse, bool recursing) { AlteredTableInfo *tab; int pass; --- 2582,2593 ---- * Traffic cop for ALTER TABLE Phase 1 operations, including simple * recursion and permission checks. * ! * Caller must have acquired appropriate lock type on relation already. * This lock should be held until commit. */ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, ! bool recurse, bool recursing, LOCKMODE lockmode) { AlteredTableInfo *tab; int pass; *************** *** 2463,2486 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, * rules. */ ATSimplePermissions(rel, true); ! ATSimpleRecursion(wqueue, rel, cmd, recurse); /* No command-specific prep needed */ pass = cmd->def ? AT_PASS_ADD_CONSTR : AT_PASS_DROP; break; case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */ ATSimplePermissions(rel, false); ! ATSimpleRecursion(wqueue, rel, cmd, recurse); /* No command-specific prep needed */ pass = AT_PASS_DROP; break; case AT_SetNotNull: /* ALTER COLUMN SET NOT NULL */ ATSimplePermissions(rel, false); ! ATSimpleRecursion(wqueue, rel, cmd, recurse); /* No command-specific prep needed */ pass = AT_PASS_ADD_CONSTR; break; case AT_SetStatistics: /* ALTER COLUMN SET STATISTICS */ ! ATSimpleRecursion(wqueue, rel, cmd, recurse); /* Performs own permission checks */ ATPrepSetStatistics(rel, cmd->name, cmd->def); pass = AT_PASS_COL_ATTRS; --- 2631,2654 ---- * rules. */ ATSimplePermissions(rel, true); ! ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); /* No command-specific prep needed */ pass = cmd->def ? AT_PASS_ADD_CONSTR : AT_PASS_DROP; break; case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */ ATSimplePermissions(rel, false); ! ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); /* No command-specific prep needed */ pass = AT_PASS_DROP; break; case AT_SetNotNull: /* ALTER COLUMN SET NOT NULL */ ATSimplePermissions(rel, false); ! ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); /* No command-specific prep needed */ pass = AT_PASS_ADD_CONSTR; break; case AT_SetStatistics: /* ALTER COLUMN SET STATISTICS */ ! ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); /* Performs own permission checks */ ATPrepSetStatistics(rel, cmd->name, cmd->def); pass = AT_PASS_COL_ATTRS; *************** *** 2493,2499 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, break; case AT_SetStorage: /* ALTER COLUMN SET STORAGE */ ATSimplePermissions(rel, false); ! ATSimpleRecursion(wqueue, rel, cmd, recurse); /* No command-specific prep needed */ pass = AT_PASS_COL_ATTRS; break; --- 2661,2667 ---- break; case AT_SetStorage: /* ALTER COLUMN SET STORAGE */ ATSimplePermissions(rel, false); ! ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); /* No command-specific prep needed */ pass = AT_PASS_COL_ATTRS; break; *************** *** 2530,2536 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, case AT_AlterColumnType: /* ALTER COLUMN TYPE */ ATSimplePermissions(rel, false); /* Performs own recursion */ ! ATPrepAlterColumnType(wqueue, tab, rel, recurse, recursing, cmd); pass = AT_PASS_ALTER_TYPE; break; case AT_ChangeOwner: /* ALTER OWNER */ --- 2698,2704 ---- case AT_AlterColumnType: /* ALTER COLUMN TYPE */ ATSimplePermissions(rel, false); /* Performs own recursion */ ! ATPrepAlterColumnType(wqueue, tab, rel, recurse, recursing, cmd, lockmode); pass = AT_PASS_ALTER_TYPE; break; case AT_ChangeOwner: /* ALTER OWNER */ *************** *** 2562,2568 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, dropCmd->subtype = AT_DropColumn; dropCmd->name = pstrdup("oid"); dropCmd->behavior = cmd->behavior; ! ATPrepCmd(wqueue, rel, dropCmd, recurse, false); } pass = AT_PASS_DROP; break; --- 2730,2736 ---- dropCmd->subtype = AT_DropColumn; dropCmd->name = pstrdup("oid"); dropCmd->behavior = cmd->behavior; ! ATPrepCmd(wqueue, rel, dropCmd, recurse, false, lockmode); } pass = AT_PASS_DROP; break; *************** *** 2617,2623 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, * conflicts). */ static void ! ATRewriteCatalogs(List **wqueue) { int pass; ListCell *ltab; --- 2785,2791 ---- * conflicts). */ static void ! ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode) { int pass; ListCell *ltab; *************** *** 2643,2654 **** ATRewriteCatalogs(List **wqueue) continue; /* ! * Exclusive lock was obtained by phase 1, needn't get it again */ rel = relation_open(tab->relid, NoLock); foreach(lcmd, subcmds) ! ATExecCmd(wqueue, tab, rel, (AlterTableCmd *) lfirst(lcmd)); /* * After the ALTER TYPE pass, do cleanup work (this is not done in --- 2811,2822 ---- continue; /* ! * Appropriate lock was obtained by phase 1, needn't get it again */ rel = relation_open(tab->relid, NoLock); foreach(lcmd, subcmds) ! ATExecCmd(wqueue, tab, rel, (AlterTableCmd *) lfirst(lcmd), lockmode); /* * After the ALTER TYPE pass, do cleanup work (this is not done in *************** *** 2683,2689 **** ATRewriteCatalogs(List **wqueue) */ static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, ! AlterTableCmd *cmd) { switch (cmd->subtype) { --- 2851,2857 ---- */ static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, ! AlterTableCmd *cmd, LOCKMODE lockmode) { switch (cmd->subtype) { *************** *** 2722,2739 **** ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, cmd->behavior, true, false, cmd->missing_ok); break; case AT_AddIndex: /* ADD INDEX */ ! ATExecAddIndex(tab, rel, (IndexStmt *) cmd->def, false); break; case AT_ReAddIndex: /* ADD INDEX */ ! ATExecAddIndex(tab, rel, (IndexStmt *) cmd->def, true); break; case AT_AddConstraint: /* ADD CONSTRAINT */ ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def, ! false); break; case AT_AddConstraintRecurse: /* ADD CONSTRAINT with recursion */ ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def, ! true); break; case AT_DropConstraint: /* DROP CONSTRAINT */ ATExecDropConstraint(rel, cmd->name, cmd->behavior, --- 2890,2907 ---- cmd->behavior, true, false, cmd->missing_ok); break; case AT_AddIndex: /* ADD INDEX */ ! ATExecAddIndex(tab, rel, (IndexStmt *) cmd->def, false, lockmode); break; case AT_ReAddIndex: /* ADD INDEX */ ! ATExecAddIndex(tab, rel, (IndexStmt *) cmd->def, true, lockmode); break; case AT_AddConstraint: /* ADD CONSTRAINT */ ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def, ! false, lockmode); break; case AT_AddConstraintRecurse: /* ADD CONSTRAINT with recursion */ ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def, ! true, lockmode); break; case AT_DropConstraint: /* DROP CONSTRAINT */ ATExecDropConstraint(rel, cmd->name, cmd->behavior, *************** *** 2971,2982 **** ATRewriteTables(List **wqueue) Relation refrel; if (rel == NULL) - { /* Long since locked, no need for another */ rel = heap_open(tab->relid, NoLock); - } ! refrel = heap_open(con->refrelid, RowShareLock); validateForeignKeyConstraint(fkconstraint, rel, refrel, con->refindid, --- 3139,3152 ---- 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); validateForeignKeyConstraint(fkconstraint, rel, refrel, con->refindid, *************** *** 3387,3393 **** ATSimplePermissionsRelationOrIndex(Relation rel) */ static void ATSimpleRecursion(List **wqueue, Relation rel, ! AlterTableCmd *cmd, bool recurse) { /* * Propagate to children if desired. Non-table relations never have --- 3557,3563 ---- */ static void ATSimpleRecursion(List **wqueue, Relation rel, ! AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode) { /* * Propagate to children if desired. Non-table relations never have *************** *** 3399,3405 **** ATSimpleRecursion(List **wqueue, Relation rel, ListCell *child; List *children; ! children = find_all_inheritors(relid, AccessExclusiveLock, NULL); /* * find_all_inheritors does the recursive search of the inheritance --- 3569,3575 ---- ListCell *child; List *children; ! children = find_all_inheritors(relid, lockmode, NULL); /* * find_all_inheritors does the recursive search of the inheritance *************** *** 3416,3422 **** ATSimpleRecursion(List **wqueue, Relation rel, /* find_all_inheritors already got lock */ childrel = relation_open(childrelid, NoLock); CheckTableNotInUse(childrel, "ALTER TABLE"); ! ATPrepCmd(wqueue, childrel, cmd, false, true); relation_close(childrel, NoLock); } } --- 3586,3592 ---- /* find_all_inheritors already got lock */ childrel = relation_open(childrelid, NoLock); CheckTableNotInUse(childrel, "ALTER TABLE"); ! ATPrepCmd(wqueue, childrel, cmd, false, true, lockmode); relation_close(childrel, NoLock); } } *************** *** 3432,3444 **** ATSimpleRecursion(List **wqueue, Relation rel, */ static void ATOneLevelRecursion(List **wqueue, Relation rel, ! AlterTableCmd *cmd) { Oid relid = RelationGetRelid(rel); ListCell *child; List *children; ! children = find_inheritance_children(relid, AccessExclusiveLock); foreach(child, children) { --- 3602,3614 ---- */ static void ATOneLevelRecursion(List **wqueue, Relation rel, ! AlterTableCmd *cmd, LOCKMODE lockmode) { Oid relid = RelationGetRelid(rel); ListCell *child; List *children; ! children = find_inheritance_children(relid, lockmode); foreach(child, children) { *************** *** 3448,3454 **** ATOneLevelRecursion(List **wqueue, Relation rel, /* find_inheritance_children already got lock */ childrel = relation_open(childrelid, NoLock); CheckTableNotInUse(childrel, "ALTER TABLE"); ! ATPrepCmd(wqueue, childrel, cmd, true, true); relation_close(childrel, NoLock); } } --- 3618,3624 ---- /* find_inheritance_children already got lock */ childrel = relation_open(childrelid, NoLock); CheckTableNotInUse(childrel, "ALTER TABLE"); ! ATPrepCmd(wqueue, childrel, cmd, true, true, lockmode); relation_close(childrel, NoLock); } } *************** *** 3584,3590 **** ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, colDefChild->inhcount = 1; colDefChild->is_local = false; ! ATOneLevelRecursion(wqueue, rel, childCmd); } else { --- 3754,3760 ---- colDefChild->inhcount = 1; colDefChild->is_local = false; ! ATOneLevelRecursion(wqueue, rel, childCmd, AccessExclusiveLock); } else { *************** *** 4392,4399 **** ATExecDropColumn(List **wqueue, Relation rel, const char *colName, * routines, we have to do this one level of recursion at a time; we can't * use find_all_inheritors to do it in one pass. */ ! children = find_inheritance_children(RelationGetRelid(rel), ! AccessExclusiveLock); if (children) { --- 4562,4568 ---- * routines, we have to do this one level of recursion at a time; we can't * use find_all_inheritors to do it in one pass. */ ! children = find_inheritance_children(RelationGetRelid(rel), AccessExclusiveLock); if (children) { *************** *** 4528,4534 **** ATExecDropColumn(List **wqueue, Relation rel, const char *colName, */ static void ATExecAddIndex(AlteredTableInfo *tab, Relation rel, ! IndexStmt *stmt, bool is_rebuild) { bool check_rights; bool skip_build; --- 4697,4703 ---- */ static void ATExecAddIndex(AlteredTableInfo *tab, Relation rel, ! IndexStmt *stmt, bool is_rebuild, LOCKMODE lockmode) { bool check_rights; bool skip_build; *************** *** 4571,4577 **** ATExecAddIndex(AlteredTableInfo *tab, Relation rel, */ static void ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ! Constraint *newConstraint, bool recurse) { Assert(IsA(newConstraint, Constraint)); --- 4740,4746 ---- */ static void ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ! Constraint *newConstraint, bool recurse, LOCKMODE lockmode) { Assert(IsA(newConstraint, Constraint)); *************** *** 4584,4590 **** ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, { case CONSTR_CHECK: ATAddCheckConstraint(wqueue, tab, rel, ! newConstraint, recurse, false); break; case CONSTR_FOREIGN: --- 4753,4759 ---- { case CONSTR_CHECK: ATAddCheckConstraint(wqueue, tab, rel, ! newConstraint, recurse, false, lockmode); break; case CONSTR_FOREIGN: *************** *** 4615,4621 **** ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, RelationGetNamespace(rel), NIL); ! ATAddForeignKeyConstraint(tab, rel, newConstraint); break; default: --- 4784,4790 ---- RelationGetNamespace(rel), NIL); ! ATAddForeignKeyConstraint(tab, rel, newConstraint, lockmode); break; default: *************** *** 4639,4645 **** ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, */ static void ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ! Constraint *constr, bool recurse, bool recursing) { List *newcons; ListCell *lcon; --- 4808,4815 ---- */ static void ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ! Constraint *constr, bool recurse, bool recursing, ! LOCKMODE lockmode) { List *newcons; ListCell *lcon; *************** *** 4694,4701 **** ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, * routines, we have to do this one level of recursion at a time; we can't * use find_all_inheritors to do it in one pass. */ ! children = find_inheritance_children(RelationGetRelid(rel), ! AccessExclusiveLock); /* * If we are told not to recurse, there had better not be any child --- 4864,4870 ---- * routines, we have to do this one level of recursion at a time; we can't * use find_all_inheritors to do it in one pass. */ ! children = find_inheritance_children(RelationGetRelid(rel), lockmode); /* * If we are told not to recurse, there had better not be any child *************** *** 4721,4727 **** ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, /* Recurse to child */ ATAddCheckConstraint(wqueue, childtab, childrel, ! constr, recurse, true); heap_close(childrel, NoLock); } --- 4890,4896 ---- /* Recurse to child */ ATAddCheckConstraint(wqueue, childtab, childrel, ! constr, recurse, true, lockmode); heap_close(childrel, NoLock); } *************** *** 4736,4742 **** ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, */ static void ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel, ! Constraint *fkconstraint) { Relation pkrel; int16 pkattnum[INDEX_MAX_KEYS]; --- 4905,4911 ---- */ static void ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel, ! Constraint *fkconstraint, LOCKMODE lockmode) { Relation pkrel; int16 pkattnum[INDEX_MAX_KEYS]; *************** *** 4753,4766 **** ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel, Oid indexOid; Oid constrOid; ! /* ! * 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 --- 4922,4928 ---- Oid indexOid; Oid constrOid; ! pkrel = heap_openrv(fkconstraint->pktable, lockmode); /* * Validity checks (permission checks wait till we have the column *************** *** 5324,5330 **** checkFkeyPermissions(Relation rel, int16 *attnums, int natts) * Scan the existing rows in a table to verify they meet a proposed FK * constraint. * ! * Caller must have opened and locked both relations. */ static void validateForeignKeyConstraint(Constraint *fkconstraint, --- 5486,5492 ---- * Scan the existing rows in a table to verify they meet a proposed FK * constraint. * ! * Caller must have opened and locked both relations appropriately. */ static void validateForeignKeyConstraint(Constraint *fkconstraint, *************** *** 5663,5670 **** ATExecDropConstraint(Relation rel, const char *constrName, * use find_all_inheritors to do it in one pass. */ if (is_check_constraint) ! children = find_inheritance_children(RelationGetRelid(rel), ! AccessExclusiveLock); else children = NIL; --- 5825,5831 ---- * use find_all_inheritors to do it in one pass. */ if (is_check_constraint) ! children = find_inheritance_children(RelationGetRelid(rel), AccessExclusiveLock); else children = NIL; *************** *** 5775,5781 **** static void ATPrepAlterColumnType(List **wqueue, AlteredTableInfo *tab, Relation rel, bool recurse, bool recursing, ! AlterTableCmd *cmd) { char *colName = cmd->name; TypeName *typeName = (TypeName *) cmd->def; --- 5936,5942 ---- ATPrepAlterColumnType(List **wqueue, AlteredTableInfo *tab, Relation rel, bool recurse, bool recursing, ! AlterTableCmd *cmd, LOCKMODE lockmode) { char *colName = cmd->name; TypeName *typeName = (TypeName *) cmd->def; *************** *** 5897,5903 **** ATPrepAlterColumnType(List **wqueue, * alter would put them out of step. */ if (recurse) ! ATSimpleRecursion(wqueue, rel, cmd, recurse); else if (!recursing && find_inheritance_children(RelationGetRelid(rel), NoLock) != NIL) ereport(ERROR, --- 6058,6064 ---- * alter would put them out of step. */ if (recurse) ! ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); else if (!recursing && find_inheritance_children(RelationGetRelid(rel), NoLock) != NIL) ereport(ERROR, *************** *** 6304,6309 **** ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab) --- 6465,6474 ---- */ } + /* + * Called to add additional work items if ALTER TYPE touched indexed + * columns. Hence we always take AccessExclusiveLocks here. + */ static void ATPostAlterTypeParse(char *cmd, List **wqueue) { *** a/src/backend/commands/trigger.c --- b/src/backend/commands/trigger.c *************** *** 141,147 **** CreateTrigger(CreateTrigStmt *stmt, const char *queryString, ObjectAddress myself, referenced; ! rel = heap_openrv(stmt->relation, AccessExclusiveLock); if (rel->rd_rel->relkind != RELKIND_RELATION) ereport(ERROR, --- 141,154 ---- 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); if (rel->rd_rel->relkind != RELKIND_RELATION) ereport(ERROR, *************** *** 417,423 **** 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. */ if (!isInternal) --- 424,430 ---- * 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 the * relation, so the trigger set won't be changing underneath us. */ if (!isInternal) *************** *** 1051,1061 **** RemoveTriggerById(Oid trigOid) elog(ERROR, "could not find tuple for trigger %u", trigOid); /* ! * Open and exclusive-lock the relation the trigger belongs to. */ relid = ((Form_pg_trigger) GETSTRUCT(tup))->tgrelid; ! rel = heap_open(relid, AccessExclusiveLock); if (rel->rd_rel->relkind != RELKIND_RELATION) ereport(ERROR, --- 1058,1071 ---- 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. */ relid = ((Form_pg_trigger) GETSTRUCT(tup))->tgrelid; ! rel = heap_open(relid, ShareRowExclusiveLock); if (rel->rd_rel->relkind != RELKIND_RELATION) ereport(ERROR, *** a/src/backend/parser/parse_utilcmd.c --- b/src/backend/parser/parse_utilcmd.c *************** *** 53,58 **** --- 53,59 ---- #include "parser/parse_utilcmd.h" #include "parser/parser.h" #include "rewrite/rewriteManip.h" + #include "storage/lock.h" #include "utils/acl.h" #include "utils/builtins.h" #include "utils/lsyscache.h" *************** *** 1528,1534 **** transformFKConstraints(ParseState *pstate, CreateStmtContext *cxt, } /* ! * transformIndexStmt - parse analysis for CREATE INDEX * * Note: this is a no-op for an index not using either index expressions or * a predicate expression. There are several code paths that create indexes --- 1529,1535 ---- } /* ! * transformIndexStmt - parse analysis for CREATE INDEX and ALTER TABLE * * Note: this is a no-op for an index not using either index expressions or * a predicate expression. There are several code paths that create indexes *************** *** 1554,1560 **** transformIndexStmt(IndexStmt *stmt, const char *queryString) * because addRangeTableEntry() would acquire only AccessShareLock, * leaving DefineIndex() needing to do a lock upgrade with consequent risk * of deadlock. Make sure this stays in sync with the type of lock ! * DefineIndex() wants. */ rel = heap_openrv(stmt->relation, (stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock)); --- 1555,1562 ---- * because addRangeTableEntry() would acquire only AccessShareLock, * leaving DefineIndex() needing to do a lock upgrade with consequent risk * of deadlock. Make sure this stays in sync with the type of lock ! * DefineIndex() wants. If we are being called by ALTER TABLE, we will ! * already hold a higher lock. */ rel = heap_openrv(stmt->relation, (stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock)); *************** *** 1919,1924 **** transformAlterTableStmt(AlterTableStmt *stmt, const char *queryString) --- 1921,1927 ---- List *newcmds = NIL; bool skipValidation = true; AlterTableCmd *newcmd; + LOCKMODE lockmode; /* * We must not scribble on the passed-in AlterTableStmt, so copy it. (This *************** *** 1927,1939 **** transformAlterTableStmt(AlterTableStmt *stmt, const char *queryString) stmt = (AlterTableStmt *) copyObject(stmt); /* ! * Acquire exclusive lock on the target relation, which will be held until * end of transaction. This ensures any decisions we make here based on * the state of the relation will still be good at execution. We must get ! * exclusive lock now because execution will; taking a lower grade lock ! * now and trying to upgrade later risks deadlock. */ ! rel = relation_openrv(stmt->relation, AccessExclusiveLock); /* Set up pstate */ pstate = make_parsestate(NULL); --- 1930,1948 ---- stmt = (AlterTableStmt *) copyObject(stmt); /* ! * Assign the appropriate lock level for this list of subcommands. ! */ ! lockmode = AlterTableGreatestLockLevel(stmt->cmds); ! ! /* ! * Acquire appropriate lock on the target relation, which will be held until * end of transaction. This ensures any decisions we make here based on * the state of the relation will still be good at execution. We must get ! * lock now because execution will later require it; taking a lower grade lock ! * now and trying to upgrade later risks deadlock. Any new commands we add ! * after this must not upgrade the lock level requested here. */ ! rel = relation_openrv(stmt->relation, lockmode); /* Set up pstate */ pstate = make_parsestate(NULL); *** a/src/backend/rewrite/rewriteDefine.c --- b/src/backend/rewrite/rewriteDefine.c *************** *** 236,246 **** 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 might be sufficient to ! * grab ShareLock to lock out insert/update/delete actions. But for now, ! * let's just grab AccessExclusiveLock all the time. */ ! event_relation = heap_open(event_relid, AccessExclusiveLock); /* * Verify relation is of a type that rules can sensibly be applied to. --- 236,249 ---- /* * 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. */ ! if (event_type == CMD_SELECT) ! event_relation = heap_open(event_relid, AccessExclusiveLock); ! else ! event_relation = heap_open(event_relid, ShareRowExclusiveLock); /* * Verify relation is of a type that rules can sensibly be applied to. *** a/src/backend/rewrite/rewriteSupport.c --- b/src/backend/rewrite/rewriteSupport.c *************** *** 70,81 **** SetRelationRuleStatus(Oid relationId, bool relHasRules, /* Do the update */ classForm->relhasrules = relHasRules; if (relIsBecomingView) classForm->relkind = RELKIND_VIEW; ! simple_heap_update(relationRelation, &tuple->t_self, tuple); ! /* Keep the catalog indexes up to date */ ! CatalogUpdateIndexes(relationRelation, tuple); } else { --- 70,85 ---- /* Do the update */ classForm->relhasrules = relHasRules; if (relIsBecomingView) + { classForm->relkind = RELKIND_VIEW; ! simple_heap_update(relationRelation, &tuple->t_self, tuple); ! /* Keep the catalog indexes up to date */ ! CatalogUpdateIndexes(relationRelation, tuple); ! } ! else ! heap_inplace_update(relationRelation, tuple); } else { *** a/src/backend/utils/adt/ri_triggers.c --- b/src/backend/utils/adt/ri_triggers.c *************** *** 2606,2612 **** RI_FKey_keyequal_upd_fk(Trigger *trigger, Relation fk_rel, * This is not a trigger procedure, but is called during ALTER TABLE * ADD FOREIGN KEY to validate the initial table contents. * ! * We expect that an exclusive lock has been taken on rel and pkrel; * hence, we do not need to lock individual rows for the check. * * If the check fails because the current user doesn't have permissions --- 2606,2612 ---- * This is not a trigger procedure, but is called during ALTER TABLE * ADD FOREIGN KEY to validate the initial table contents. * ! * We expect that a ShareRowExclusiveLock or higher has been taken on rel and pkrel; * hence, we do not need to lock individual rows for the check. * * If the check fails because the current user doesn't have permissions *** a/src/include/commands/tablecmds.h --- b/src/include/commands/tablecmds.h *************** *** 15,20 **** --- 15,21 ---- #define TABLECMDS_H #include "nodes/parsenodes.h" + #include "storage/lock.h" #include "utils/relcache.h" *************** *** 24,29 **** extern void RemoveRelations(DropStmt *drop); --- 25,32 ---- extern void AlterTable(AlterTableStmt *stmt); + extern LOCKMODE AlterTableGreatestLockLevel(List *cmds); + extern void ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing); extern void AlterTableInternal(Oid relid, List *cmds, bool recurse);