Back-patch the 8.3 fix that prohibits TRUNCATE, CLUSTER, and REINDEX when the
authorTom Lane <[email protected]>
Tue, 27 May 2008 21:13:39 +0000 (21:13 +0000)
committerTom Lane <[email protected]>
Tue, 27 May 2008 21:13:39 +0000 (21:13 +0000)
current transaction has any open references to the target relation or index
(implying it has an active query using the relation).  Also back-patch the
8.2 fix that prohibits TRUNCATE and CLUSTER when there are pending
AFTER-trigger events.  Per suggestion from Heikki.

src/backend/catalog/index.c
src/backend/commands/cluster.c
src/backend/commands/tablecmds.c
src/backend/commands/trigger.c
src/include/commands/tablecmds.h
src/include/commands/trigger.h

index e0e73bfd8330c8932b44ab8317009c6fe0ad75bb..1054602ba42e46d37db408589cfa293c0caf7f09 100644 (file)
 #include "catalog/heap.h"
 #include "catalog/index.h"
 #include "catalog/indexing.h"
+#include "catalog/namespace.h"
 #include "catalog/pg_constraint.h"
 #include "catalog/pg_opclass.h"
 #include "catalog/pg_type.h"
+#include "commands/tablecmds.h"
 #include "executor/executor.h"
 #include "miscadmin.h"
 #include "optimizer/clauses.h"
@@ -1672,6 +1674,21 @@ reindex_index(Oid indexId)
        iRel = index_open(indexId);
        LockRelation(iRel, AccessExclusiveLock);
 
+       /*
+        * Don't allow reindex on temp tables of other backends ... their local
+        * buffer manager is not going to cope.
+        */
+       if (isOtherTempNamespace(RelationGetNamespace(iRel)))
+               ereport(ERROR,
+                               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                errmsg("cannot reindex temporary tables of other sessions")));
+
+       /*
+        * Also check for active uses of the index in the current transaction;
+        * we don't want to reindex underneath an open indexscan.
+        */
+       CheckTableNotInUse(iRel, "REINDEX INDEX");
+
        /*
         * If it's a shared index, we must do inplace processing (because we have
         * no way to update relfilenode in other databases).  Otherwise we can do
index c47fa6c21b0fda5f6a952eaf9fce6820c9bef42c..f45d3852cfae1f40b7c433c27634f711230f0fa0 100644 (file)
@@ -419,6 +419,12 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck)
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                           errmsg("cannot cluster temporary tables of other sessions")));
 
+       /*
+        * Also check for active uses of the relation in the current transaction,
+        * including open scans and pending AFTER trigger events.
+        */
+       CheckTableNotInUse(OldHeap, "CLUSTER");
+
        /* Drop relcache refcnt on OldIndex, but keep lock */
        index_close(OldIndex);
 }
index 952875c8dbb4972837dd2cfae08cb54a3757ac6d..a9b42ea35a264ab512f352262cb95c5db30b2322 100644 (file)
@@ -588,6 +588,12 @@ ExecuteTruncate(List *relations)
                                        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                          errmsg("cannot truncate temporary tables of other sessions")));
 
+               /*
+                * Also check for active uses of the relation in the current
+                * transaction, including open scans and pending AFTER trigger events.
+                */
+               CheckTableNotInUse(rel, "TRUNCATE");
+
                /* Save it into the list of rels to truncate */
                rels = lappend(rels, rel);
        }
@@ -1763,6 +1769,55 @@ update_ri_trigger_args(Oid relid,
        CommandCounterIncrement();
 }
 
+/*
+ * 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
+ * possibly be relaxed to only error out for certain types of alterations.
+ * But the use-case for allowing any of these things is not obvious, so we
+ * won't work hard at it for now.
+ *
+ * We also reject these commands if there are any pending AFTER trigger events
+ * for the rel.  This is certainly necessary for the rewriting variants of
+ * ALTER TABLE, because they don't preserve tuple TIDs and so the pending
+ * events would try to fetch the wrong tuples.  It might be overly cautious
+ * in other cases, but again it seems better to err on the side of paranoia.
+ *
+ * REINDEX calls this with "rel" referencing the index to be rebuilt; here
+ * we are worried about active indexscans on the index.  The trigger-event
+ * check can be skipped, since we are doing no damage to the parent table.
+ *
+ * The statement name (eg, "ALTER TABLE") is passed for use in error messages.
+ */
+void
+CheckTableNotInUse(Relation rel, const char *stmt)
+{
+       int                     expected_refcnt;
+
+       expected_refcnt = rel->rd_isnailed ? 2 : 1;
+       if (rel->rd_refcnt != expected_refcnt)
+               ereport(ERROR,
+                               (errcode(ERRCODE_OBJECT_IN_USE),
+                                /* translator: first %s is a SQL command, eg ALTER TABLE */
+                                errmsg("cannot %s \"%s\" because "
+                                               "it is being used by active queries in this session",
+                                               stmt, RelationGetRelationName(rel))));
+
+       if (rel->rd_rel->relkind != RELKIND_INDEX &&
+               AfterTriggerPendingOnRel(RelationGetRelid(rel)))
+               ereport(ERROR,
+                               (errcode(ERRCODE_OBJECT_IN_USE),
+                                /* translator: first %s is a SQL command, eg ALTER TABLE */
+                                errmsg("cannot %s \"%s\" because "
+                                               "it has pending trigger events",
+                                               stmt, RelationGetRelationName(rel))));
+}
+
 /*
  * AlterTable
  *             Execute ALTER TABLE, which can be a list of subcommands
@@ -1800,26 +1855,8 @@ void
 AlterTable(AlterTableStmt *stmt)
 {
        Relation rel = relation_openrv(stmt->relation, AccessExclusiveLock);
-       int                     expected_refcnt;
 
-       /*
-        * Disallow ALTER TABLE when the current backend has any open reference
-        * to it besides the one we just got (such as an open cursor or active
-        * plan); our AccessExclusiveLock doesn't protect us against stomping on
-        * our own foot, only other people's feet!
-        *
-        * Note: the only case known to cause serious trouble is ALTER COLUMN TYPE,
-        * and some changes are obviously pretty benign, so this could possibly
-        * be relaxed to only error out for certain types of alterations.  But
-        * the use-case for allowing any of these things is not obvious, so we
-        * won't work hard at it for now.
-        */
-       expected_refcnt = rel->rd_isnailed ? 2 : 1;
-       if (rel->rd_refcnt != expected_refcnt)
-               ereport(ERROR,
-                               (errcode(ERRCODE_OBJECT_IN_USE),
-                                errmsg("relation \"%s\" is being used by active queries in this session",
-                                               RelationGetRelationName(rel))));
+       CheckTableNotInUse(rel, "ALTER TABLE");
 
        ATController(rel, stmt->cmds, interpretInhOption(stmt->relation->inhOpt));
 }
@@ -1832,7 +1869,8 @@ AlterTable(AlterTableStmt *stmt)
  * We do not reject if the relation is already open, because it's quite
  * likely that one or more layers of caller have it open.  That means it
  * is unsafe to use this entry point for alterations that could break
- * existing query plans.
+ * existing query plans.  On the assumption it's not used for such, we
+ * don't have to reject pending AFTER triggers, either.
  */
 void
 AlterTableInternal(Oid relid, List *cmds, bool recurse)
@@ -2744,12 +2782,7 @@ ATSimpleRecursion(List **wqueue, Relation rel,
                        if (childrelid == relid)
                                continue;
                        childrel = relation_open(childrelid, AccessExclusiveLock);
-                       /* check for child relation in use in this session */
-                       if (childrel->rd_refcnt != 1)
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_OBJECT_IN_USE),
-                                                errmsg("relation \"%s\" is being used by active queries in this session",
-                                                               RelationGetRelationName(childrel))));
+                       CheckTableNotInUse(childrel, "ALTER TABLE");
                        ATPrepCmd(wqueue, childrel, cmd, false, true);
                        relation_close(childrel, NoLock);
                }
@@ -2781,12 +2814,7 @@ ATOneLevelRecursion(List **wqueue, Relation rel,
                Relation        childrel;
 
                childrel = relation_open(childrelid, AccessExclusiveLock);
-               /* check for child relation in use in this session */
-               if (childrel->rd_refcnt != 1)
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_OBJECT_IN_USE),
-                                        errmsg("relation \"%s\" is being used by active queries in this session",
-                                                       RelationGetRelationName(childrel))));
+               CheckTableNotInUse(childrel, "ALTER TABLE");
                ATPrepCmd(wqueue, childrel, cmd, true, true);
                relation_close(childrel, NoLock);
        }
@@ -3613,12 +3641,7 @@ ATExecDropColumn(Relation rel, const char *colName,
                        Form_pg_attribute childatt;
 
                        childrel = heap_open(childrelid, AccessExclusiveLock);
-                       /* check for child relation in use in this session */
-                       if (childrel->rd_refcnt != 1)
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_OBJECT_IN_USE),
-                                                errmsg("relation \"%s\" is being used by active queries in this session",
-                                                               RelationGetRelationName(childrel))));
+                       CheckTableNotInUse(childrel, "ALTER TABLE");
 
                        tuple = SearchSysCacheCopyAttName(childrelid, colName);
                        if (!HeapTupleIsValid(tuple))           /* shouldn't happen */
index f8f4a4dcddfe79972d11bb7258850c7cf1b336b9..53afef02fc2037912842dcd1346109334875ca12 100644 (file)
@@ -3062,6 +3062,70 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt)
        }
 }
 
+/* ----------
+ * AfterTriggerPendingOnRel()
+ *             Test to see if there are any pending after-trigger events for rel.
+ *
+ * This is used by TRUNCATE, CLUSTER, ALTER TABLE, etc to detect whether
+ * it is unsafe to perform major surgery on a relation.  Note that only
+ * local pending events are examined.  We assume that having exclusive lock
+ * on a rel guarantees there are no unserviced events in other backends ---
+ * but having a lock does not prevent there being such events in our own.
+ *
+ * In some scenarios it'd be reasonable to remove pending events (more
+ * specifically, mark them DONE by the current subxact) but without a lot
+ * of knowledge of the trigger semantics we can't do this in general.
+ * ----------
+ */
+bool
+AfterTriggerPendingOnRel(Oid relid)
+{
+       AfterTriggerEvent event;
+       int                     depth;
+
+       /* No-op if we aren't in a transaction.  (Shouldn't happen?) */
+       if (afterTriggers == NULL)
+               return false;
+
+       /* Scan queued events */
+       for (event = afterTriggers->events.head;
+                event != NULL;
+                event = event->ate_next)
+       {
+               /*
+                * We can ignore completed events.  (Even if a DONE flag is rolled
+                * back by subxact abort, it's OK because the effects of the TRUNCATE
+                * or whatever must get rolled back too.)
+                */
+               if (event->ate_event & AFTER_TRIGGER_DONE)
+                       continue;
+
+               if (event->ate_relid == relid)
+                       return true;
+       }
+
+       /*
+        * Also scan events queued by incomplete queries.  This could only matter
+        * if TRUNCATE/etc is executed by a function or trigger within an updating
+        * query on the same relation, which is pretty perverse, but let's check.
+        */
+       for (depth = 0; depth <= afterTriggers->query_depth; depth++)
+       {
+               for (event = afterTriggers->query_stack[depth].head;
+                        event != NULL;
+                        event = event->ate_next)
+               {
+                       if (event->ate_event & AFTER_TRIGGER_DONE)
+                               continue;
+
+                       if (event->ate_relid == relid)
+                               return true;
+               }
+       }
+
+       return false;
+}
+
 
 /* ----------
  * AfterTriggerSaveEvent()
index 8ebac07e9f863f6604f68ca75adce5e0b7b696fc..ff0b38f066716c3626af986af1eafa3409acf343 100644 (file)
@@ -34,6 +34,8 @@ extern void AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
                                                           Oid oldNspOid, Oid newNspOid,
                                                           bool hasDependEntry);
 
+extern void CheckTableNotInUse(Relation rel, const char *stmt);
+
 extern void ExecuteTruncate(List *relations);
 
 extern void renameatt(Oid myrelid,
index 0cd9c01de41f71ec361a9b1d49fbdfa9a566f9cd..82fe2762530cdc1741aab33f764a2d4befd1d65a 100644 (file)
@@ -164,8 +164,8 @@ extern void AfterTriggerFireDeferred(void);
 extern void AfterTriggerEndXact(bool isCommit);
 extern void AfterTriggerBeginSubXact(void);
 extern void AfterTriggerEndSubXact(bool isCommit);
-
 extern void AfterTriggerSetState(ConstraintsSetStmt *stmt);
+extern bool AfterTriggerPendingOnRel(Oid relid);
 
 
 /*