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

Commit3085993

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 parent20b4819 commit3085993

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
@@ -3723,13 +3723,6 @@ afterTriggerCopyBitmap(Bitmapset *src)
37233723
if (src==NULL)
37243724
returnNULL;
37253725

3726-
/* Create event context if we didn't already */
3727-
if (afterTriggers.event_cxt==NULL)
3728-
afterTriggers.event_cxt=
3729-
AllocSetContextCreate(TopTransactionContext,
3730-
"AfterTriggerEvents",
3731-
ALLOCSET_DEFAULT_SIZES);
3732-
37333726
oldcxt=MemoryContextSwitchTo(afterTriggers.event_cxt);
37343727

37353728
dst=bms_copy(src);
@@ -3831,16 +3824,21 @@ afterTriggerAddEvent(AfterTriggerEventList *events,
38313824
(char*)newshared >=chunk->endfree;
38323825
newshared--)
38333826
{
3827+
/* compare fields roughly by probability of them being different */
38343828
if (newshared->ats_tgoid==evtshared->ats_tgoid&&
3835-
newshared->ats_relid==evtshared->ats_relid&&
38363829
newshared->ats_event==evtshared->ats_event&&
3830+
newshared->ats_firing_id==0&&
38373831
newshared->ats_table==evtshared->ats_table&&
3838-
newshared->ats_firing_id==0)
3832+
newshared->ats_relid==evtshared->ats_relid&&
3833+
bms_equal(newshared->ats_modifiedcols,
3834+
evtshared->ats_modifiedcols))
38393835
break;
38403836
}
38413837
if ((char*)newshared<chunk->endfree)
38423838
{
38433839
*newshared=*evtshared;
3840+
/* now we must make a suitably-long-lived copy of the bitmap */
3841+
newshared->ats_modifiedcols=afterTriggerCopyBitmap(evtshared->ats_modifiedcols);
38443842
newshared->ats_firing_id=0;/* just to be sure */
38453843
chunk->endfree= (char*)newshared;
38463844
}
@@ -5857,7 +5855,7 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
58575855
new_shared.ats_table=transition_capture->tcs_private;
58585856
else
58595857
new_shared.ats_table=NULL;
5860-
new_shared.ats_modifiedcols=afterTriggerCopyBitmap(modifiedCols);
5858+
new_shared.ats_modifiedcols=modifiedCols;
58615859

58625860
afterTriggerAddEvent(&afterTriggers.query_stack[afterTriggers.query_depth].events,
58635861
&new_event,&new_shared);

0 commit comments

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