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

Commit2b72fed

Browse files
committed
Repair incorrect handling of AfterTriggerSharedData.ats_modifiedcols.
This patch fixes two distinct errors that both ultimately traceto commit71d60e2, which added the ats_modifiedcols field.The more severe error is that ats_modifiedcols wasn't accounted forin afterTriggerAddEvent's scanning loop that looks for a pre-existingduplicate AfterTriggerSharedData. Thus, a new event could beincorrectly matched to an AfterTriggerSharedData that has a differentvalue of ats_modifiedcols, resulting in the wrong tg_updatedcolsbitmap getting passed to the trigger whenever it finally gets fired.We'd not noticed because (a) few triggers consult tg_updatedcols,and (b) we had no tests exercising a case where such a trigger wascalled as an AFTER trigger. In the test case added by this commit,contrib/lo's trigger fails to remove a large object when expectedbecause (without this fix) it thinks the LO OID column hasn't changed.The other problem was introduced by commitce5aaea, which copied themodified-columns bitmap into trigger-related storage. It made a copyfor every trigger event, whereas what we really want is to make a newcopy only when we make a new AfterTriggerSharedData entry. (We couldimagine adding extra logic to reduce the number of bitmap copies stillmore, but it doesn't look worthwhile at the moment.) In a simple testof an UPDATE of 10000000 rows with a single AFTER trigger, this thinkoroughly tripled the amount of memory consumed by the pending-triggersdata structures, from 160446744 to 480443440 bytes.Fixing the first problem requires introducing a bms_equal() call intoafterTriggerAddEvent's scanning loop, which is slightly annoying froma speed perspective. However, getting rid of the excessive bms_copy()calls from the second problem balances that out; overall speed oftrigger operations is the same or slightly better, in my tests.Discussion:https://postgr.es/m/3496294.1737501591@sss.pgh.pa.usBackpatch-through: 13
1 parentdbc3499 commit2b72fed

File tree

3 files changed

+117
-10
lines changed

3 files changed

+117
-10
lines changed

‎contrib/lo/expected/lo.out

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,73 @@ SELECT lo_get(43214);
4747
DELETE FROM image;
4848
SELECT lo_get(43214);
4949
ERROR: large object 43214 does not exist
50+
-- Now let's try it with an AFTER trigger
51+
DROP TRIGGER t_raster ON image;
52+
CREATE CONSTRAINT TRIGGER t_raster AFTER UPDATE OR DELETE ON image
53+
DEFERRABLE INITIALLY DEFERRED
54+
FOR EACH ROW EXECUTE PROCEDURE lo_manage(raster);
55+
SELECT lo_create(43223);
56+
lo_create
57+
-----------
58+
43223
59+
(1 row)
60+
61+
SELECT lo_create(43224);
62+
lo_create
63+
-----------
64+
43224
65+
(1 row)
66+
67+
SELECT lo_create(43225);
68+
lo_create
69+
-----------
70+
43225
71+
(1 row)
72+
73+
INSERT INTO image (title, raster) VALUES ('beautiful image', 43223);
74+
SELECT lo_get(43223);
75+
lo_get
76+
--------
77+
\x
78+
(1 row)
79+
80+
UPDATE image SET raster = 43224 WHERE title = 'beautiful image';
81+
SELECT lo_get(43223); -- gone
82+
ERROR: large object 43223 does not exist
83+
SELECT lo_get(43224);
84+
lo_get
85+
--------
86+
\x
87+
(1 row)
88+
89+
-- test updating of unrelated column
90+
UPDATE image SET title = 'beautiful picture' WHERE title = 'beautiful image';
91+
SELECT lo_get(43224);
92+
lo_get
93+
--------
94+
\x
95+
(1 row)
96+
97+
-- this case used to be buggy
98+
BEGIN;
99+
UPDATE image SET title = 'beautiful image' WHERE title = 'beautiful picture';
100+
UPDATE image SET raster = 43225 WHERE title = 'beautiful image';
101+
SELECT lo_get(43224);
102+
lo_get
103+
--------
104+
\x
105+
(1 row)
106+
107+
COMMIT;
108+
SELECT lo_get(43224); -- gone
109+
ERROR: large object 43224 does not exist
110+
SELECT lo_get(43225);
111+
lo_get
112+
--------
113+
\x
114+
(1 row)
115+
116+
DELETE FROM image;
117+
SELECT lo_get(43225); -- gone
118+
ERROR: large object 43225 does not exist
50119
DROP TABLE image;

‎contrib/lo/sql/lo.sql

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,44 @@ DELETE FROM image;
2727

2828
SELECT lo_get(43214);
2929

30+
-- Now let's try it with an AFTER trigger
31+
32+
DROPTRIGGER t_rasterON image;
33+
34+
CREATECONSTRAINT TRIGGER t_raster AFTERUPDATEORDELETEON image
35+
DEFERRABLE INITIALLY DEFERRED
36+
FOR EACH ROW EXECUTE PROCEDURE lo_manage(raster);
37+
38+
SELECT lo_create(43223);
39+
SELECT lo_create(43224);
40+
SELECT lo_create(43225);
41+
42+
INSERT INTO image (title, raster)VALUES ('beautiful image',43223);
43+
44+
SELECT lo_get(43223);
45+
46+
UPDATE imageSET raster=43224WHERE title='beautiful image';
47+
48+
SELECT lo_get(43223);-- gone
49+
SELECT lo_get(43224);
50+
51+
-- test updating of unrelated column
52+
UPDATE imageSET title='beautiful picture'WHERE title='beautiful image';
53+
54+
SELECT lo_get(43224);
55+
56+
-- this case used to be buggy
57+
BEGIN;
58+
UPDATE imageSET title='beautiful image'WHERE title='beautiful picture';
59+
UPDATE imageSET raster=43225WHERE title='beautiful image';
60+
SELECT lo_get(43224);
61+
COMMIT;
62+
63+
SELECT lo_get(43224);-- gone
64+
SELECT lo_get(43225);
65+
66+
DELETEFROM image;
67+
68+
SELECT lo_get(43225);-- gone
69+
3070
DROPTABLE image;

‎src/backend/commands/trigger.c

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4003,13 +4003,6 @@ afterTriggerCopyBitmap(Bitmapset *src)
40034003
if (src==NULL)
40044004
returnNULL;
40054005

4006-
/* Create event context if we didn't already */
4007-
if (afterTriggers.event_cxt==NULL)
4008-
afterTriggers.event_cxt=
4009-
AllocSetContextCreate(TopTransactionContext,
4010-
"AfterTriggerEvents",
4011-
ALLOCSET_DEFAULT_SIZES);
4012-
40134006
oldcxt=MemoryContextSwitchTo(afterTriggers.event_cxt);
40144007

40154008
dst=bms_copy(src);
@@ -4111,16 +4104,21 @@ afterTriggerAddEvent(AfterTriggerEventList *events,
41114104
(char*)newshared >=chunk->endfree;
41124105
newshared--)
41134106
{
4107+
/* compare fields roughly by probability of them being different */
41144108
if (newshared->ats_tgoid==evtshared->ats_tgoid&&
4115-
newshared->ats_relid==evtshared->ats_relid&&
41164109
newshared->ats_event==evtshared->ats_event&&
4110+
newshared->ats_firing_id==0&&
41174111
newshared->ats_table==evtshared->ats_table&&
4118-
newshared->ats_firing_id==0)
4112+
newshared->ats_relid==evtshared->ats_relid&&
4113+
bms_equal(newshared->ats_modifiedcols,
4114+
evtshared->ats_modifiedcols))
41194115
break;
41204116
}
41214117
if ((char*)newshared<chunk->endfree)
41224118
{
41234119
*newshared=*evtshared;
4120+
/* now we must make a suitably-long-lived copy of the bitmap */
4121+
newshared->ats_modifiedcols=afterTriggerCopyBitmap(evtshared->ats_modifiedcols);
41244122
newshared->ats_firing_id=0;/* just to be sure */
41254123
chunk->endfree= (char*)newshared;
41264124
}
@@ -6431,7 +6429,7 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
64316429
new_shared.ats_table=transition_capture->tcs_private;
64326430
else
64336431
new_shared.ats_table=NULL;
6434-
new_shared.ats_modifiedcols=afterTriggerCopyBitmap(modifiedCols);
6432+
new_shared.ats_modifiedcols=modifiedCols;
64356433

64366434
afterTriggerAddEvent(&afterTriggers.query_stack[afterTriggers.query_depth].events,
64376435
&new_event,&new_shared);

0 commit comments

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