Finish disabling reduced-lock-levels-for-DDL feature.

Previous patch only covered the ALTER TABLE changes, not changes in other
commands; and it neglected to revert the documentation changes.

diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml
index 1d337b80550d1b72b6b871017f393555b976a9a8..1ec07dd9eeed23d46642f59c3f9791afce7ca06f 100644 (file)
--- 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
         </para>
 
         <para>
-         Acquired by <command>CREATE TRIGGER</command>,
-         <command>CREATE RULE</command> (except for <literal>ON SELECT</>
-         rules) and some forms of <command>ALTER TABLE</command>.
+         This lock mode is not automatically acquired by any
+         <productname>PostgreSQL</productname> command.
         </para>
        </listitem>
       </varlistentry>
@@ -925,10 +924,10 @@ ERROR:  could not serialize access due to read/write dependencies among transact
         </para>
 
         <para>
-         Acquired by the <command>DROP TABLE</command>,
+         Acquired by the <command>ALTER TABLE</>, <command>DROP TABLE</>,
          <command>TRUNCATE</command>, <command>REINDEX</command>,
          <command>CLUSTER</command>, and <command>VACUUM FULL</command>
-         commands, and some forms of <command>ALTER TABLE</>.
+         commands.
          This is also the default lock mode for <command>LOCK TABLE</command>
          statements that do not specify a mode explicitly.
         </para>

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3bc350a11342bd65eb1712313b403bde494ee10d..c1af12a5a304743f8866c550be6e91037b336c5a 100644 (file)
--- 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 8072c7768797deadd6098a3a4b8cb8eba52f4a6e..9cf8372e9674f0452f7d37f84833e445e816ad78 100644 (file)
--- 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 bb7d468d19db7d43b682c2308eb534bf728c4d17..60b988175bc96a7dc6a96b5350cf3871e02747e2 100644 (file)
--- 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.