From 290d4b37ab98be20ebb79c658b7590b8982d7f01 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 27 May 2008 21:13:39 +0000 Subject: [PATCH] Back-patch the 8.3 fix that prohibits TRUNCATE, CLUSTER, and REINDEX when the 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 | 19 +++++- src/backend/commands/cluster.c | 8 ++- src/backend/commands/tablecmds.c | 101 +++++++++++++++++++------------ src/backend/commands/trigger.c | 66 +++++++++++++++++++- src/include/commands/tablecmds.h | 4 +- src/include/commands/trigger.h | 4 +- 6 files changed, 157 insertions(+), 45 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 1e355532e0..343779cfad 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.261.2.4 2008/01/03 21:24:26 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.261.2.5 2008/05/27 21:13:39 tgl Exp $ * * * INTERFACE ROUTINES @@ -31,9 +31,11 @@ #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 diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 698609a428..9f18c919c1 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.141.2.2 2007/09/12 15:16:20 alvherre Exp $ + * $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.141.2.3 2008/05/27 21:13:39 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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); } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index e0f8786507..1350b0d026 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.174.2.6 2008/05/09 22:37:47 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.174.2.7 2008/05/27 21:13:39 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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 */ diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 9510f84900..276efb8477 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.195.2.4 2007/08/15 19:16:03 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.195.2.5 2008/05/27 21:13:39 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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() diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index 0774164844..7e00b9ea8b 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.24 2005/10/15 02:49:44 momjian Exp $ + * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.24.2.1 2008/05/27 21:13:39 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(List *relations); extern void renameatt(Oid myrelid, diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index 39325a1903..464f8d49f5 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.56 2005/10/15 02:49:44 momjian Exp $ + * $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.56.2.1 2008/05/27 21:13:39 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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); /* -- 2.40.0