Reduce lock levels of CREATE TRIGGER and some ALTER TABLE, CREATE RULE actions.

Avoid hard-coding lockmode used for many altering DDL commands, allowing easier
future changes of lock levels. Implementation of initial analysis on DDL
sub-commands, so that many lock levels are now at ShareUpdateExclusiveLock or
ShareRowExclusiveLock, allowing certain DDL not to block reads/writes.
First of number of planned changes in this area; additional docs required
when full project complete.

doc/src/sgml/mvcc.sgml

@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/mvcc.sgml,v 2.75 2010/05/03 15:35:30 alvherre Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/mvcc.sgml,v 2.76 2010/07/28 05:22:24 sriggs Exp $ -->
 
  <chapter id="mvcc">
   <title>Concurrency Control</title>
@@ -532,7 +532,7 @@ SELECT SUM(value) FROM mytab WHERE class = 2;
     most <productname>PostgreSQL</productname> 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, <command>ALTER TABLE</> cannot safely be
+    command executes.  (For example, <command>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.)
    </para>
@@ -695,8 +695,9 @@ SELECT SUM(value) FROM mytab WHERE class = 2;
         </para>
 
         <para>
-         This lock mode is not automatically acquired by any
+         Acquired by <command>CREATE TRIGGER</command>,
-         <productname>PostgreSQL</productname> command.
+         <command>CREATE RULE</command> (except for <literal>ON SELECT</>
+         rules) and in some cases <command>ALTER TABLE</command>.
         </para>
        </listitem>
       </varlistentry>
@@ -742,11 +743,12 @@ SELECT SUM(value) FROM mytab WHERE class = 2;
         </para>
 
         <para>
-         Acquired by the <command>ALTER TABLE</command>, <command>DROP
+         Acquired by the <command>DROP TABLE</command>,
-         TABLE</command>, <command>TRUNCATE</command>, <command>REINDEX</command>,
+         <command>TRUNCATE</command>, <command>REINDEX</command>,
          <command>CLUSTER</command>, and <command>VACUUM FULL</command>
-         commands.  This is also the default lock mode for <command>LOCK
+         commands, as well as most variants of <command>ALTER TABLE</>.
-         TABLE</command> statements that do not specify a mode explicitly.
+         This is also the default lock mode for <command>LOCK TABLE</command>
+         statements that do not specify a mode explicitly.
         </para>
        </listitem>
       </varlistentry>

src/backend/catalog/pg_shdepend.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/pg_shdepend.c,v 1.43 2010/07/06 19:18:56 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/pg_shdepend.c,v 1.44 2010/07/28 05:22:24 sriggs Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1346,7 +1346,7 @@ shdepReassignOwned(List *roleids, Oid newrole)
 					 * owned sequences, etc when we happen to visit them
 					 * before their parent table.
 					 */
-					ATExecChangeOwner(sdepForm->objid, newrole, true);
+					ATExecChangeOwner(sdepForm->objid, newrole, true, AccessExclusiveLock);
 					break;
 
 				case ProcedureRelationId:

src/backend/commands/alter.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/alter.c,v 1.36 2010/06/13 17:43:12 rhaas Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/alter.c,v 1.37 2010/07/28 05:22:24 sriggs Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -190,7 +190,7 @@ ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt)
 		case OBJECT_VIEW:
 			CheckRelationOwnership(stmt->relation, true);
 			AlterTableNamespace(stmt->relation, stmt->newschema,
-								stmt->objectType);
+								stmt->objectType, AccessExclusiveLock);
 			break;
 
 		case OBJECT_TYPE:

src/backend/commands/trigger.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.262 2010/02/26 02:00:39 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.263 2010/07/28 05:22:24 sriggs Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -141,7 +141,14 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
 	ObjectAddress myself,
 				referenced;
 
-	rel = heap_openrv(stmt->relation, AccessExclusiveLock);
+	/*
+	 * 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,7 +424,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 AccessExclusiveLock on the
+	 * 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,11 +1058,14 @@ 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.  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, AccessExclusiveLock);
+	rel = heap_open(relid, ShareRowExclusiveLock);
 
 	if (rel->rd_rel->relkind != RELKIND_RELATION)
 		ereport(ERROR,

src/backend/commands/typecmds.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.149 2010/07/25 23:21:21 rhaas Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.150 2010/07/28 05:22:24 sriggs Exp $
  *
  * DESCRIPTION
  *	  The "DefineFoo" routines take the parse tree and pick out the
@@ -2638,7 +2638,7 @@ AlterTypeOwner(List *names, Oid newOwnerId)
 		 * AlterTypeOwnerInternal to take care of the pg_type entry(s).
 		 */
 		if (typTup->typtype == TYPTYPE_COMPOSITE)
-			ATExecChangeOwner(typTup->typrelid, newOwnerId, true);
+			ATExecChangeOwner(typTup->typrelid, newOwnerId, true, AccessExclusiveLock);
 		else
 		{
 			/*

src/backend/parser/parse_utilcmd.c

@@ -19,7 +19,7 @@
  * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.40 2010/02/26 02:00:53 momjian Exp $
+ *	$PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.41 2010/07/28 05:22:24 sriggs Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -53,6 +53,7 @@
 #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,7 +1529,7 @@ transformFKConstraints(ParseState *pstate, CreateStmtContext *cxt,
 }
 
 /*
- * transformIndexStmt - parse analysis for CREATE INDEX
+ * 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,7 +1555,8 @@ 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.
+	 * 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,6 +1921,7 @@ transformAlterTableStmt(AlterTableStmt *stmt, const char *queryString)
 	List	   *newcmds = NIL;
 	bool		skipValidation = true;
 	AlterTableCmd *newcmd;
+	LOCKMODE	lockmode;
 
 	/*
 	 * We must not scribble on the passed-in AlterTableStmt, so copy it. (This
@@ -1927,13 +1930,19 @@ transformAlterTableStmt(AlterTableStmt *stmt, const char *queryString)
 	stmt = (AlterTableStmt *) copyObject(stmt);
 
 	/*
-	 * Acquire exclusive lock on the target relation, which will be held until
+	 * Assign the appropriate lock level for this list of subcommands.
+	 */
+	lockmode = AlterTableGetLockLevel(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
-	 * exclusive lock now because execution will; taking a lower grade lock
+	 * lock now because execution will later require it; taking a lower grade lock
-	 * now and trying to upgrade later risks deadlock.
+	 * 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, AccessExclusiveLock);
+	rel = relation_openrv(stmt->relation, lockmode);
 
 	/* Set up pstate */
 	pstate = make_parsestate(NULL);

src/backend/rewrite/rewriteDefine.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.141 2010/02/14 18:42:15 rhaas Exp $
+ *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.142 2010/07/28 05:22:24 sriggs Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -236,11 +236,14 @@ 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
+	 * event relation.	For other types of rules, it is sufficient to
-	 * grab ShareLock to lock out insert/update/delete actions.  But for now,
+	 * grab ShareRowExclusiveLock to lock out insert/update/delete actions
-	 * let's just grab AccessExclusiveLock all the time.
+	 * and to ensure that we lock out current CREATE RULE statements.
 	 */
-	event_relation = heap_open(event_relid, AccessExclusiveLock);
+	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.

src/backend/utils/adt/ri_triggers.c

@@ -15,7 +15,7 @@
  *
  * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.119 2010/07/22 00:47:52 rhaas Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.120 2010/07/28 05:22:24 sriggs Exp $
  *
  * ----------
  */
@@ -2608,7 +2608,7 @@ 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;
+ *	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

src/include/commands/tablecmds.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.46 2010/02/01 19:28:56 rhaas Exp $
+ * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.47 2010/07/28 05:22:24 sriggs Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -15,6 +15,7 @@
 #define TABLECMDS_H
 
 #include "nodes/parsenodes.h"
+#include "storage/lock.h"
 #include "utils/relcache.h"
 
 
@@ -24,12 +25,14 @@ extern void RemoveRelations(DropStmt *drop);
 
 extern void AlterTable(AlterTableStmt *stmt);
 
-extern void ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing);
+extern LOCKMODE AlterTableGetLockLevel(List *cmds);
+
+extern void ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing, LOCKMODE lockmode);
 
 extern void AlterTableInternal(Oid relid, List *cmds, bool recurse);
 
 extern void AlterTableNamespace(RangeVar *relation, const char *newschema,
-					ObjectType stmttype);
+					ObjectType stmttype, LOCKMODE lockmode);
 
 extern void AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
 							   Oid oldNspOid, Oid newNspOid,