]> granicus.if.org Git - postgresql/commitdiff
Back-patch the 8.3 fix that prohibits TRUNCATE, CLUSTER, and REINDEX when the
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 27 May 2008 21:13:25 +0000 (21:13 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 27 May 2008 21:13:25 +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 dcc4aa0a98ca7b5dc845d61ddfb1f9939f172b71..34a5b97f04540bc3d14c92397b97d983c3225b79 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.274.2.2 2008/01/03 21:23:45 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.274.2.3 2008/05/27 21:13:25 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
 #include "catalog/heap.h"
 #include "catalog/index.h"
 #include "catalog/indexing.h"
+#include "catalog/namespace.h"
 #include "catalog/pg_constraint.h"
 #include "catalog/pg_operator.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"
@@ -1954,6 +1956,21 @@ reindex_index(Oid indexId)
         */
        iRel = index_open(indexId, 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 f0825a9d02fbb293389d375c8dcde32064cd6bf9..9164641b74818f8f336e9a76b4027606dca7b70f 100644 (file)
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.154.2.2 2007/09/29 18:05:28 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.154.2.3 2008/05/27 21:13:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -28,6 +28,7 @@
 #include "catalog/namespace.h"
 #include "catalog/toasting.h"
 #include "commands/cluster.h"
+#include "commands/tablecmds.h"
 #include "miscadmin.h"
 #include "utils/acl.h"
 #include "utils/fmgroids.h"
@@ -449,6 +450,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, NoLock);
 }
index a9a13a803e558030367f9e36e60d923c47e264a4..3006f0a36271f3fb26d1ba89759f97f520fbca14 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.206.2.5 2008/05/09 22:37:41 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.206.2.6 2008/05/27 21:13:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -596,13 +596,6 @@ ExecuteTruncate(TruncateStmt *stmt)
                heap_truncate_check_FKs(rels, false);
 #endif
 
-       /*
-        * Also check for pending AFTER trigger events on the target relations. We
-        * can't just leave those be, since they will try to fetch tuples that the
-        * TRUNCATE removes.
-        */
-       AfterTriggerCheckTruncate(relids);
-
        /*
         * OK, truncate each table.
         */
@@ -683,6 +676,12 @@ truncate_check_rel(Relation rel)
                ereport(ERROR,
                                (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");
 }
 
 /*----------
@@ -1912,6 +1911,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
@@ -1949,26 +1997,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));
 }
@@ -1981,7 +2011,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)
@@ -2939,12 +2970,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);
                }
@@ -2976,12 +3002,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);
        }
@@ -3799,12 +3820,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 47de3ca6fc2285dde65e4ebb3f8535d02e0f8ca2..75e0cf150d83e0cea1571d01c3e6b73d14bcfba5 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.210.2.4 2007/08/15 19:15:55 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.210.2.5 2008/05/27 21:13:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3190,28 +3190,29 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt)
 }
 
 /* ----------
- * AfterTriggerCheckTruncate()
- *             Test deferred-trigger status to see if a TRUNCATE is OK.
+ * AfterTriggerPendingOnRel()
+ *             Test to see if there are any pending after-trigger events for rel.
  *
- * The argument is a list of OIDs of relations due to be truncated.
- * We raise error if there are any pending after-trigger events for them.
+ * 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.
  * ----------
  */
-void
-AfterTriggerCheckTruncate(List *relids)
+bool
+AfterTriggerPendingOnRel(Oid relid)
 {
        AfterTriggerEvent event;
        int                     depth;
 
-       /*
-        * Ignore call if we aren't in a transaction.  (Shouldn't happen?)
-        */
+       /* No-op if we aren't in a transaction.  (Shouldn't happen?) */
        if (afterTriggers == NULL)
-               return;
+               return false;
 
        /* Scan queued events */
        for (event = afterTriggers->events.head;
@@ -3221,21 +3222,18 @@ AfterTriggerCheckTruncate(List *relids)
                /*
                 * 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
-                * must get rolled back too.)
+                * or whatever must get rolled back too.)
                 */
                if (event->ate_event & AFTER_TRIGGER_DONE)
                        continue;
 
-               if (list_member_oid(relids, event->ate_relid))
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                                        errmsg("cannot truncate table \"%s\" because it has pending trigger events",
-                                                       get_rel_name(event->ate_relid))));
+               if (event->ate_relid == relid)
+                       return true;
        }
 
        /*
         * Also scan events queued by incomplete queries.  This could only matter
-        * if a TRUNCATE is executed by a function or trigger within an updating
+        * 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++)
@@ -3247,13 +3245,12 @@ AfterTriggerCheckTruncate(List *relids)
                        if (event->ate_event & AFTER_TRIGGER_DONE)
                                continue;
 
-                       if (list_member_oid(relids, event->ate_relid))
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                                                errmsg("cannot truncate table \"%s\" because it has pending trigger events",
-                                                               get_rel_name(event->ate_relid))));
+                       if (event->ate_relid == relid)
+                               return true;
                }
        }
+
+       return false;
 }
 
 
index f50a21577aee0feaa3c6e06b57ec6c7c9b4831c5..f2e2b2778964ab7202541f5d43af51e1a1fa4788 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.31.2.1 2007/05/11 20:18:17 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.31.2.2 2008/05/27 21:13:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -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(TruncateStmt *stmt);
 
 extern void renameatt(Oid myrelid,
index 31253d8b4fe1561ce8c0b3c3e94fc717e75a0386..d88f584885410cd28c129d38ebf6ac1500118cae 100644 (file)
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.59 2006/09/04 21:15:56 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.59.2.1 2008/05/27 21:13:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -165,7 +165,7 @@ extern void AfterTriggerEndXact(bool isCommit);
 extern void AfterTriggerBeginSubXact(void);
 extern void AfterTriggerEndSubXact(bool isCommit);
 extern void AfterTriggerSetState(ConstraintsSetStmt *stmt);
-extern void AfterTriggerCheckTruncate(List *relids);
+extern bool AfterTriggerPendingOnRel(Oid relid);
 
 
 /*