Finish disabling reduced-lock-levels-for-DDL feature.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 7 Jul 2011 17:14:46 +0000 (13:14 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 7 Jul 2011 17:15:15 +0000 (13:15 -0400)
Previous patch only covered the ALTER TABLE changes, not changes in other
commands; and it neglected to revert the documentation changes.

doc/src/sgml/mvcc.sgml
src/backend/commands/tablecmds.c
src/backend/commands/trigger.c
src/backend/rewrite/rewriteDefine.c

index 1d337b80550d1b72b6b871017f393555b976a9a8..1ec07dd9eeed23d46642f59c3f9791afce7ca06f 100644 (file)
@@ -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>
index 3bc350a11342bd65eb1712313b403bde494ee10d..c1af12a5a304743f8866c550be6e91037b336c5a 100644 (file)
@@ -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
index 8072c7768797deadd6098a3a4b8cb8eba52f4a6e..9cf8372e9674f0452f7d37f84833e445e816ad78 100644 (file)
@@ -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)
index bb7d468d19db7d43b682c2308eb534bf728c4d17..60b988175bc96a7dc6a96b5350cf3871e02747e2 100644 (file)
@@ -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.