From 2c3d9db56d5d49bdc777b174982251c01348e3d8 Mon Sep 17 00:00:00 2001
From: Simon Riggs <simon@2ndQuadrant.com>
Date: Mon, 4 Jul 2011 09:31:40 +0100
Subject: [PATCH] Reset ALTER TABLE lock levels to AccessExclusiveLock in all
 cases. Locks on inheritance parent remain at lower level, as they were
 before. Remove entry from 9.1 release notes.

---
 doc/src/sgml/release-9.1.sgml    | 17 +----------------
 src/backend/commands/tablecmds.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/doc/src/sgml/release-9.1.sgml b/doc/src/sgml/release-9.1.sgml
index 9dfca1c786..914464eb6c 100644
--- a/doc/src/sgml/release-9.1.sgml
+++ b/doc/src/sgml/release-9.1.sgml
@@ -846,8 +846,7 @@
 
       <listitem>
        <para>
-        Add functions to control streaming replication replay (Simon
-        Riggs)
+        Add functions to control streaming replication replay (Simon Riggs)
        </para>
 
        <para>
@@ -1742,20 +1741,6 @@
        </para>
       </listitem>
 
-      <listitem>
-       <para>
-        Minimize lock levels for <link
-        linkend="SQL-CREATETRIGGER"><command>CREATE TRIGGER</></link>
-        and many <link linkend="SQL-ALTERTABLE"><command>ALTER
-        TABLE</></link> and <link linkend="SQL-CREATERULE"><command>CREATE
-        RULE</></link> operations (Simon Riggs)
-       </para>
-
-       <para>
-        This improves database availability when altering active databases.
-       </para>
-      </listitem>
-
      </itemizedlist>
 
     </sect4>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a3a99d2880..22864de957 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2578,6 +2578,31 @@ AlterTableInternal(Oid relid, List *cmds, bool recurse)
 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 are 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.
+	 */
+#ifdef REDUCED_ALTER_TABLE_LOCK_LEVELS
 	ListCell   *lcmd;
 	LOCKMODE	lockmode = ShareUpdateExclusiveLock;
 
@@ -2726,6 +2751,9 @@ AlterTableGetLockLevel(List *cmds)
 		if (cmd_lockmode > lockmode)
 			lockmode = cmd_lockmode;
 	}
+#else
+	LOCKMODE	lockmode = AccessExclusiveLock;
+#endif
 
 	return lockmode;
 }
-- 
2.40.0