Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit20e8621

Browse files
committed
Forbid ALTER TABLE and CLUSTER when there are pending AFTER-trigger events
in the current backend for the target table. These operations move tuplesaround and would thus invalidate the TIDs stored in the trigger event records.(We need not worry about events in other backends, since acquiring exclusivelock should be enough to ensure there aren't any.) It might be sufficientto forbid only the table-rewriting variants of ALTER TABLE, but in the absenceof any compelling use-case, let's just be safe and simple. Per follow-oninvestigation of bug #3847, though this is not actually the same problemreported therein.Possibly this should be back-patched, but since the case has never beenreported from the field, I didn't bother.
1 parent69528ae commit20e8621

File tree

4 files changed

+85
-66
lines changed

4 files changed

+85
-66
lines changed

‎src/backend/commands/cluster.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*
1212
*
1313
* IDENTIFICATION
14-
* $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.166 2008/01/01 19:45:48 momjian Exp $
14+
* $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.167 2008/01/02 23:34:42 tgl Exp $
1515
*
1616
*-------------------------------------------------------------------------
1717
*/
@@ -30,6 +30,7 @@
3030
#include"catalog/namespace.h"
3131
#include"catalog/toasting.h"
3232
#include"commands/cluster.h"
33+
#include"commands/trigger.h"
3334
#include"commands/vacuum.h"
3435
#include"miscadmin.h"
3536
#include"storage/procarray.h"
@@ -457,6 +458,17 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck)
457458
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
458459
errmsg("cannot cluster temporary tables of other sessions")));
459460

461+
/*
462+
* Also check for pending AFTER trigger events on the relation. We can't
463+
* just leave those be, since they will try to fetch tuples that the
464+
* CLUSTER has moved to new TIDs.
465+
*/
466+
if (AfterTriggerPendingOnRel(RelationGetRelid(OldHeap)))
467+
ereport(ERROR,
468+
(errcode(ERRCODE_OBJECT_IN_USE),
469+
errmsg("cannot cluster table \"%s\" because it has pending trigger events",
470+
RelationGetRelationName(OldHeap))));
471+
460472
/* Drop relcache refcnt on OldIndex, but keep lock */
461473
index_close(OldIndex,NoLock);
462474
}

‎src/backend/commands/tablecmds.c

Lines changed: 50 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.238 2008/01/01 19:45:49 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.239 2008/01/02 23:34:42 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -194,6 +194,7 @@ static void validateForeignKeyConstraint(FkConstraint *fkconstraint,
194194
Relationrel,Relationpkrel,OidconstraintOid);
195195
staticvoidcreateForeignKeyTriggers(Relationrel,FkConstraint*fkconstraint,
196196
OidconstraintOid);
197+
staticvoidCheckTableNotInUse(Relationrel);
197198
staticvoidATController(Relationrel,List*cmds,boolrecurse);
198199
staticvoidATPrepCmd(List**wqueue,Relationrel,AlterTableCmd*cmd,
199200
boolrecurse,boolrecursing);
@@ -598,13 +599,6 @@ ExecuteTruncate(TruncateStmt *stmt)
598599
heap_truncate_check_FKs(rels, false);
599600
#endif
600601

601-
/*
602-
* Also check for pending AFTER trigger events on the target relations. We
603-
* can't just leave those be, since they will try to fetch tuples that the
604-
* TRUNCATE removes.
605-
*/
606-
AfterTriggerCheckTruncate(relids);
607-
608602
/*
609603
* OK, truncate each table.
610604
*/
@@ -685,6 +679,17 @@ truncate_check_rel(Relation rel)
685679
ereport(ERROR,
686680
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
687681
errmsg("cannot truncate temporary tables of other sessions")));
682+
683+
/*
684+
* Also check for pending AFTER trigger events on the relation. We can't
685+
* just leave those be, since they will try to fetch tuples that the
686+
* TRUNCATE removes.
687+
*/
688+
if (AfterTriggerPendingOnRel(RelationGetRelid(rel)))
689+
ereport(ERROR,
690+
(errcode(ERRCODE_OBJECT_IN_USE),
691+
errmsg("cannot truncate table \"%s\" because it has pending trigger events",
692+
RelationGetRelationName(rel))));
688693
}
689694

690695
/*----------
@@ -1749,28 +1754,47 @@ void
17491754
AlterTable(AlterTableStmt*stmt)
17501755
{
17511756
Relationrel=relation_openrv(stmt->relation,AccessExclusiveLock);
1757+
1758+
CheckTableNotInUse(rel);
1759+
1760+
ATController(rel,stmt->cmds,interpretInhOption(stmt->relation->inhOpt));
1761+
}
1762+
1763+
/*
1764+
* Disallow ALTER TABLE when the current backend has any open reference to
1765+
* it besides the one we just got (such as an open cursor or active plan);
1766+
* our AccessExclusiveLock doesn't protect us against stomping on our own
1767+
* foot, only other people's feet!
1768+
*
1769+
* Note: the only case known to cause serious trouble is ALTER COLUMN TYPE,
1770+
* and some changes are obviously pretty benign, so this could possibly be
1771+
* relaxed to only error out for certain types of alterations. But the
1772+
* use-case for allowing any of these things is not obvious, so we won't
1773+
* work hard at it for now.
1774+
*
1775+
* We also reject ALTER TABLE if there are any pending AFTER trigger events
1776+
* for the rel. This is certainly necessary for the rewriting variants of
1777+
* ALTER TABLE, because they don't preserve tuple TIDs and so the pending
1778+
* events would try to fetch the wrong tuples. It might be overly cautious
1779+
* in other cases, but again it seems better to err on the side of paranoia.
1780+
*/
1781+
staticvoid
1782+
CheckTableNotInUse(Relationrel)
1783+
{
17521784
intexpected_refcnt;
17531785

1754-
/*
1755-
* Disallow ALTER TABLE when the current backend has any open reference to
1756-
* it besides the one we just got (such as an open cursor or active plan);
1757-
* our AccessExclusiveLock doesn't protect us against stomping on our own
1758-
* foot, only other people's feet!
1759-
*
1760-
* Note: the only case known to cause serious trouble is ALTER COLUMN
1761-
* TYPE, and some changes are obviously pretty benign, so this could
1762-
* possibly be relaxed to only error out for certain types of alterations.
1763-
* But the use-case for allowing any of these things is not obvious, so we
1764-
* won't work hard at it for now.
1765-
*/
17661786
expected_refcnt=rel->rd_isnailed ?2 :1;
17671787
if (rel->rd_refcnt!=expected_refcnt)
17681788
ereport(ERROR,
17691789
(errcode(ERRCODE_OBJECT_IN_USE),
17701790
errmsg("relation \"%s\" is being used by active queries in this session",
17711791
RelationGetRelationName(rel))));
17721792

1773-
ATController(rel,stmt->cmds,interpretInhOption(stmt->relation->inhOpt));
1793+
if (AfterTriggerPendingOnRel(RelationGetRelid(rel)))
1794+
ereport(ERROR,
1795+
(errcode(ERRCODE_OBJECT_IN_USE),
1796+
errmsg("cannot alter table \"%s\" because it has pending trigger events",
1797+
RelationGetRelationName(rel))));
17741798
}
17751799

17761800
/*
@@ -1781,7 +1805,8 @@ AlterTable(AlterTableStmt *stmt)
17811805
* We do not reject if the relation is already open, because it's quite
17821806
* likely that one or more layers of caller have it open. That means it
17831807
* is unsafe to use this entry point for alterations that could break
1784-
* existing query plans.
1808+
* existing query plans. On the assumption it's not used for such, we
1809+
* don't have to reject pending AFTER triggers, either.
17851810
*/
17861811
void
17871812
AlterTableInternal(Oidrelid,List*cmds,boolrecurse)
@@ -2784,12 +2809,7 @@ ATSimpleRecursion(List **wqueue, Relation rel,
27842809
if (childrelid==relid)
27852810
continue;
27862811
childrel=relation_open(childrelid,AccessExclusiveLock);
2787-
/* check for child relation in use in this session */
2788-
if (childrel->rd_refcnt!=1)
2789-
ereport(ERROR,
2790-
(errcode(ERRCODE_OBJECT_IN_USE),
2791-
errmsg("relation \"%s\" is being used by active queries in this session",
2792-
RelationGetRelationName(childrel))));
2812+
CheckTableNotInUse(childrel);
27932813
ATPrepCmd(wqueue,childrel,cmd, false, true);
27942814
relation_close(childrel,NoLock);
27952815
}
@@ -2821,12 +2841,7 @@ ATOneLevelRecursion(List **wqueue, Relation rel,
28212841
Relationchildrel;
28222842

28232843
childrel=relation_open(childrelid,AccessExclusiveLock);
2824-
/* check for child relation in use in this session */
2825-
if (childrel->rd_refcnt!=1)
2826-
ereport(ERROR,
2827-
(errcode(ERRCODE_OBJECT_IN_USE),
2828-
errmsg("relation \"%s\" is being used by active queries in this session",
2829-
RelationGetRelationName(childrel))));
2844+
CheckTableNotInUse(childrel);
28302845
ATPrepCmd(wqueue,childrel,cmd, true, true);
28312846
relation_close(childrel,NoLock);
28322847
}
@@ -3655,12 +3670,7 @@ ATExecDropColumn(Relation rel, const char *colName,
36553670
Form_pg_attributechildatt;
36563671

36573672
childrel=heap_open(childrelid,AccessExclusiveLock);
3658-
/* check for child relation in use in this session */
3659-
if (childrel->rd_refcnt!=1)
3660-
ereport(ERROR,
3661-
(errcode(ERRCODE_OBJECT_IN_USE),
3662-
errmsg("relation \"%s\" is being used by active queries in this session",
3663-
RelationGetRelationName(childrel))));
3673+
CheckTableNotInUse(childrel);
36643674

36653675
tuple=SearchSysCacheCopyAttName(childrelid,colName);
36663676
if (!HeapTupleIsValid(tuple))/* shouldn't happen */

‎src/backend/commands/trigger.c

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.226 2008/01/01 19:45:49 momjian Exp $
10+
* $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.227 2008/01/02 23:34:42 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -3478,28 +3478,29 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt)
34783478
}
34793479

34803480
/* ----------
3481-
*AfterTriggerCheckTruncate()
3482-
*Testdeferred-trigger statusto see ifa TRUNCATE is OK.
3481+
*AfterTriggerPendingOnRel()
3482+
*Test to see ifthere are any pending after-trigger events for rel.
34833483
*
3484-
* The argument is a list of OIDs of relations due to be truncated.
3485-
* We raise error if there are any pending after-trigger events for them.
3484+
* This is used by TRUNCATE, CLUSTER, ALTER TABLE, etc to detect whether
3485+
* it is unsafe to perform major surgery on a relation. Note that only
3486+
* local pending events are examined. We assume that having exclusive lock
3487+
* on a rel guarantees there are no unserviced events in other backends ---
3488+
* but having a lock does not prevent there being such events in our own.
34863489
*
34873490
* In some scenarios it'd be reasonable to remove pending events (more
34883491
* specifically, mark them DONE by the current subxact) but without a lot
34893492
* of knowledge of the trigger semantics we can't do this in general.
34903493
* ----------
34913494
*/
3492-
void
3493-
AfterTriggerCheckTruncate(List*relids)
3495+
bool
3496+
AfterTriggerPendingOnRel(Oidrelid)
34943497
{
34953498
AfterTriggerEventevent;
34963499
intdepth;
34973500

3498-
/*
3499-
* Ignore call if we aren't in a transaction. (Shouldn't happen?)
3500-
*/
3501+
/* No-op if we aren't in a transaction. (Shouldn't happen?) */
35013502
if (afterTriggers==NULL)
3502-
return;
3503+
return false;
35033504

35043505
/* Scan queued events */
35053506
for (event=afterTriggers->events.head;
@@ -3509,21 +3510,18 @@ AfterTriggerCheckTruncate(List *relids)
35093510
/*
35103511
* We can ignore completed events.(Even if a DONE flag is rolled
35113512
* back by subxact abort, it's OK because the effects of the TRUNCATE
3512-
* must get rolled back too.)
3513+
*or whatevermust get rolled back too.)
35133514
*/
35143515
if (event->ate_event&AFTER_TRIGGER_DONE)
35153516
continue;
35163517

3517-
if (list_member_oid(relids,event->ate_relid))
3518-
ereport(ERROR,
3519-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
3520-
errmsg("cannot truncate table \"%s\" because it has pending trigger events",
3521-
get_rel_name(event->ate_relid))));
3518+
if (event->ate_relid==relid)
3519+
return true;
35223520
}
35233521

35243522
/*
35253523
* Also scan events queued by incomplete queries. This could only matter
3526-
* ifaTRUNCATE is executed by a function or trigger within an updating
3524+
* if TRUNCATE/etc is executed by a function or trigger within an updating
35273525
* query on the same relation, which is pretty perverse, but let's check.
35283526
*/
35293527
for (depth=0;depth <=afterTriggers->query_depth;depth++)
@@ -3535,13 +3533,12 @@ AfterTriggerCheckTruncate(List *relids)
35353533
if (event->ate_event&AFTER_TRIGGER_DONE)
35363534
continue;
35373535

3538-
if (list_member_oid(relids,event->ate_relid))
3539-
ereport(ERROR,
3540-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
3541-
errmsg("cannot truncate table \"%s\" because it has pending trigger events",
3542-
get_rel_name(event->ate_relid))));
3536+
if (event->ate_relid==relid)
3537+
return true;
35433538
}
35443539
}
3540+
3541+
return false;
35453542
}
35463543

35473544

‎src/include/commands/trigger.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.65 2008/01/01 19:45:57 momjian Exp $
9+
* $PostgreSQL: pgsql/src/include/commands/trigger.h,v 1.66 2008/01/02 23:34:42 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -149,7 +149,7 @@ extern void AfterTriggerEndXact(bool isCommit);
149149
externvoidAfterTriggerBeginSubXact(void);
150150
externvoidAfterTriggerEndSubXact(boolisCommit);
151151
externvoidAfterTriggerSetState(ConstraintsSetStmt*stmt);
152-
externvoidAfterTriggerCheckTruncate(List*relids);
152+
externboolAfterTriggerPendingOnRel(Oidrelid);
153153

154154

155155
/*

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp