diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 9ba6077464a..5176e8d6ace 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.166 2008/01/01 19:45:48 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.167 2008/01/02 23:34:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -30,6 +30,7 @@ #include "catalog/namespace.h" #include "catalog/toasting.h" #include "commands/cluster.h" +#include "commands/trigger.h" #include "commands/vacuum.h" #include "miscadmin.h" #include "storage/procarray.h" @@ -457,6 +458,17 @@ 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 pending AFTER trigger events on the relation. We can't + * just leave those be, since they will try to fetch tuples that the + * CLUSTER has moved to new TIDs. + */ + if (AfterTriggerPendingOnRel(RelationGetRelid(OldHeap))) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_IN_USE), + errmsg("cannot cluster table \"%s\" because it has pending trigger events", + RelationGetRelationName(OldHeap)))); + /* Drop relcache refcnt on OldIndex, but keep lock */ index_close(OldIndex, NoLock); } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index cd2324a7c67..301420c401d 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.238 2008/01/01 19:45:49 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.239 2008/01/02 23:34:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -194,6 +194,7 @@ static void validateForeignKeyConstraint(FkConstraint *fkconstraint, Relation rel, Relation pkrel, Oid constraintOid); static void createForeignKeyTriggers(Relation rel, FkConstraint *fkconstraint, Oid constraintOid); +static void CheckTableNotInUse(Relation rel); static void ATController(Relation rel, List *cmds, bool recurse); static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, bool recurse, bool recursing); @@ -598,13 +599,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. */ @@ -685,6 +679,17 @@ truncate_check_rel(Relation rel) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot truncate temporary tables of other sessions"))); + + /* + * Also check for pending AFTER trigger events on the relation. We can't + * just leave those be, since they will try to fetch tuples that the + * TRUNCATE removes. + */ + if (AfterTriggerPendingOnRel(RelationGetRelid(rel))) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_IN_USE), + errmsg("cannot truncate table \"%s\" because it has pending trigger events", + RelationGetRelationName(rel)))); } /*---------- @@ -1749,20 +1754,35 @@ void AlterTable(AlterTableStmt *stmt) { Relation rel = relation_openrv(stmt->relation, AccessExclusiveLock); + + CheckTableNotInUse(rel); + + ATController(rel, stmt->cmds, interpretInhOption(stmt->relation->inhOpt)); +} + +/* + * 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. + * + * We also reject ALTER TABLE 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. + */ +static void +CheckTableNotInUse(Relation rel) +{ 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, @@ -1770,7 +1790,11 @@ AlterTable(AlterTableStmt *stmt) errmsg("relation \"%s\" is being used by active queries in this session", RelationGetRelationName(rel)))); - ATController(rel, stmt->cmds, interpretInhOption(stmt->relation->inhOpt)); + if (AfterTriggerPendingOnRel(RelationGetRelid(rel))) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_IN_USE), + errmsg("cannot alter table \"%s\" because it has pending trigger events", + RelationGetRelationName(rel)))); } /* @@ -1781,7 +1805,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) @@ -2784,12 +2809,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); ATPrepCmd(wqueue, childrel, cmd, false, true); relation_close(childrel, NoLock); } @@ -2821,12 +2841,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); ATPrepCmd(wqueue, childrel, cmd, true, true); relation_close(childrel, NoLock); } @@ -3655,12 +3670,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); 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 7fca0cd5af0..5bca2233279 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.226 2008/01/01 19:45:49 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.227 2008/01/02 23:34:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -3478,28 +3478,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; @@ -3509,21 +3510,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++) @@ -3535,13 +3533,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; } diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index f09e8d9387c..5e0dda4744d 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.65 2008/01/01 19:45:57 momjian Exp $ + * $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.66 2008/01/02 23:34:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -149,7 +149,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); /*