diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 1e355532e0b..343779cfad1 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 698609a428e..9f18c919c16 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 e0f87865078..1350b0d0269 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 9510f849006..276efb8477e 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 07741648448..7e00b9ea8bf 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 39325a19032..464f8d49f53 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); /*