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

Commit94ec005

Browse files
committed
In INSERT/UPDATE, use the table's real tuple descriptor as target.
This back-patches commit20d3fe9 into the v12 and v13 branches.At the time I thought that commit was not fixing any observablebug, but Bertrand Drouvot showed otherwise: adding a dropped columnto the previously-considered scenario crashes v12 and v13, unless thedropped column happens to be an integer. That is, of course, becausethe tupdesc we derive from the plan output tlist fails to describethe dropped column accurately, so that we'll do the wrong thing witha tuple in which that column isn't NULL.There is no bug in pre-v12 branches because they already did usethe table's real tuple descriptor for any trigger-returned tuple.It seems that this set of bugs can be blamed on the changes thatremoved es_trig_tuple_slot, though I've not attempted to pin thatdown precisely.Although there's no code change needed in HEAD, update the test caseto include a dropped column there too.Discussion:https://postgr.es/m/db5d97c8-f48a-51e2-7b08-b73d5434d425@amazon.comDiscussion:https://postgr.es/m/16644-5da7ef98a7ac4545@postgresql.org
1 parent16eadc4 commit94ec005

File tree

6 files changed

+102
-66
lines changed

6 files changed

+102
-66
lines changed

‎src/backend/commands/trigger.c

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,6 @@ static bool GetTupleForTrigger(EState *estate,
8989
LockTupleModelockmode,
9090
TupleTableSlot*oldslot,
9191
TupleTableSlot**newSlot);
92-
staticHeapTupleMaterializeTupleForTrigger(TupleTableSlot*slot,
93-
bool*shouldFree);
9492
staticboolTriggerEnabled(EState*estate,ResultRelInfo*relinfo,
9593
Trigger*trigger,TriggerEventevent,
9694
Bitmapset*modifiedCols,
@@ -3036,7 +3034,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
30363034
ExecCopySlot(newslot,epqslot_clean);
30373035
}
30383036

3039-
trigtuple=MaterializeTupleForTrigger(oldslot,&should_free_trig);
3037+
trigtuple=ExecFetchSlotHeapTuple(oldslot, true,&should_free_trig);
30403038
}
30413039
else
30423040
{
@@ -3404,40 +3402,6 @@ GetTupleForTrigger(EState *estate,
34043402
return true;
34053403
}
34063404

3407-
/*
3408-
* Extract a HeapTuple that we can pass off to trigger functions.
3409-
*
3410-
* We must materialize the tuple and make sure it is not dependent on any
3411-
* attrmissing data. This is needed for the old row in BEFORE UPDATE
3412-
* triggers, since they can choose to pass back this exact tuple as the update
3413-
* result, causing the tuple to be inserted into an executor slot that lacks
3414-
* the attrmissing data.
3415-
*
3416-
* Currently we don't seem to need to remove the attrmissing dependency in any
3417-
* other cases, but keep this as a separate function to simplify fixing things
3418-
* if that changes.
3419-
*/
3420-
staticHeapTuple
3421-
MaterializeTupleForTrigger(TupleTableSlot*slot,bool*shouldFree)
3422-
{
3423-
HeapTupletup;
3424-
TupleDesctupdesc=slot->tts_tupleDescriptor;
3425-
3426-
tup=ExecFetchSlotHeapTuple(slot, true,shouldFree);
3427-
if (HeapTupleHeaderGetNatts(tup->t_data)<tupdesc->natts&&
3428-
tupdesc->constr&&tupdesc->constr->missing)
3429-
{
3430-
HeapTuplenewtup;
3431-
3432-
newtup=heap_expand_tuple(tup,tupdesc);
3433-
if (*shouldFree)
3434-
heap_freetuple(tup);
3435-
*shouldFree= true;
3436-
tup=newtup;
3437-
}
3438-
returntup;
3439-
}
3440-
34413405
/*
34423406
* Is trigger enabled to fire?
34433407
*/

‎src/backend/executor/execJunk.c

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,23 +54,48 @@
5454
*
5555
* The source targetlist is passed in. The output tuple descriptor is
5656
* built from the non-junk tlist entries.
57-
* An optional resultSlot can be passed as well.
57+
* An optional resultSlot can be passed as well; otherwise, we create one.
5858
*/
5959
JunkFilter*
6060
ExecInitJunkFilter(List*targetList,TupleTableSlot*slot)
6161
{
62-
JunkFilter*junkfilter;
6362
TupleDesccleanTupType;
64-
intcleanLength;
65-
AttrNumber*cleanMap;
66-
ListCell*t;
67-
AttrNumbercleanResno;
6863

6964
/*
7065
* Compute the tuple descriptor for the cleaned tuple.
7166
*/
7267
cleanTupType=ExecCleanTypeFromTL(targetList);
7368

69+
/*
70+
* The rest is the same as ExecInitJunkFilterInsertion, ie, we want to map
71+
* every non-junk targetlist column into the output tuple.
72+
*/
73+
returnExecInitJunkFilterInsertion(targetList,cleanTupType,slot);
74+
}
75+
76+
/*
77+
* ExecInitJunkFilterInsertion
78+
*
79+
* Initialize a JunkFilter for insertions into a table.
80+
*
81+
* Here, we are given the target "clean" tuple descriptor rather than
82+
* inferring it from the targetlist. Although the target descriptor can
83+
* contain deleted columns, that is not of concern here, since the targetlist
84+
* should contain corresponding NULL constants (cf. ExecCheckPlanOutput).
85+
* It is assumed that the caller has checked that the table's columns match up
86+
* with the non-junk columns of the targetlist.
87+
*/
88+
JunkFilter*
89+
ExecInitJunkFilterInsertion(List*targetList,
90+
TupleDesccleanTupType,
91+
TupleTableSlot*slot)
92+
{
93+
JunkFilter*junkfilter;
94+
intcleanLength;
95+
AttrNumber*cleanMap;
96+
ListCell*t;
97+
AttrNumbercleanResno;
98+
7499
/*
75100
* Use the given slot, or make a new slot if we weren't given one.
76101
*/
@@ -93,17 +118,18 @@ ExecInitJunkFilter(List *targetList, TupleTableSlot *slot)
93118
if (cleanLength>0)
94119
{
95120
cleanMap= (AttrNumber*)palloc(cleanLength*sizeof(AttrNumber));
96-
cleanResno=1;
121+
cleanResno=0;
97122
foreach(t,targetList)
98123
{
99124
TargetEntry*tle=lfirst(t);
100125

101126
if (!tle->resjunk)
102127
{
103-
cleanMap[cleanResno-1]=tle->resno;
128+
cleanMap[cleanResno]=tle->resno;
104129
cleanResno++;
105130
}
106131
}
132+
Assert(cleanResno==cleanLength);
107133
}
108134
else
109135
cleanMap=NULL;

‎src/backend/executor/nodeModifyTable.c

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2660,15 +2660,27 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
26602660
TupleTableSlot*junkresslot;
26612661

26622662
subplan=mtstate->mt_plans[i]->plan;
2663-
if (operation==CMD_INSERT||operation==CMD_UPDATE)
2664-
ExecCheckPlanOutput(resultRelInfo->ri_RelationDesc,
2665-
subplan->targetlist);
26662663

26672664
junkresslot=
26682665
ExecInitExtraTupleSlot(estate,NULL,
26692666
table_slot_callbacks(resultRelInfo->ri_RelationDesc));
2670-
j=ExecInitJunkFilter(subplan->targetlist,
2671-
junkresslot);
2667+
2668+
/*
2669+
* For an INSERT or UPDATE, the result tuple must always match
2670+
* the target table's descriptor. For a DELETE, it won't
2671+
* (indeed, there's probably no non-junk output columns).
2672+
*/
2673+
if (operation==CMD_INSERT||operation==CMD_UPDATE)
2674+
{
2675+
ExecCheckPlanOutput(resultRelInfo->ri_RelationDesc,
2676+
subplan->targetlist);
2677+
j=ExecInitJunkFilterInsertion(subplan->targetlist,
2678+
RelationGetDescr(resultRelInfo->ri_RelationDesc),
2679+
junkresslot);
2680+
}
2681+
else
2682+
j=ExecInitJunkFilter(subplan->targetlist,
2683+
junkresslot);
26722684

26732685
if (operation==CMD_UPDATE||operation==CMD_DELETE)
26742686
{

‎src/include/executor/executor.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,9 @@ extern void ResetTupleHashTable(TupleHashTable hashtable);
150150
*/
151151
externJunkFilter*ExecInitJunkFilter(List*targetList,
152152
TupleTableSlot*slot);
153+
externJunkFilter*ExecInitJunkFilterInsertion(List*targetList,
154+
TupleDesccleanTupType,
155+
TupleTableSlot*slot);
153156
externJunkFilter*ExecInitJunkFilterConversion(List*targetList,
154157
TupleDesccleanTupType,
155158
TupleTableSlot*slot);

‎src/test/regress/expected/triggers.out

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -216,34 +216,56 @@ select * from trigtest;
216216

217217
drop table trigtest;
218218
-- Check behavior with an implicit column default, too (bug #16644)
219-
create table trigtest (a integer);
219+
create table trigtest (
220+
a integer,
221+
b bool default true not null,
222+
c text default 'xyzzy' not null);
220223
create trigger trigger_return_old
221224
before insert or delete or update on trigtest
222225
for each row execute procedure trigger_return_old();
223226
insert into trigtest values(1);
224227
select * from trigtest;
225-
a
226-
---
227-
1
228+
a | b | c
229+
---+---+-------
230+
1 | t | xyzzy
231+
(1 row)
232+
233+
alter table trigtest add column d integer default 42 not null;
234+
select * from trigtest;
235+
a | b | c | d
236+
---+---+-------+----
237+
1 | t | xyzzy | 42
238+
(1 row)
239+
240+
update trigtest set a = 2 where a = 1 returning *;
241+
a | b | c | d
242+
---+---+-------+----
243+
1 | t | xyzzy | 42
244+
(1 row)
245+
246+
select * from trigtest;
247+
a | b | c | d
248+
---+---+-------+----
249+
1 | t | xyzzy | 42
228250
(1 row)
229251

230-
alter table trigtestadd column b integer default 42 not null;
252+
alter table trigtestdrop column b;
231253
select * from trigtest;
232-
a |b
233-
---+----
234-
1 | 42
254+
a | c | d
255+
---+-------+----
256+
1 |xyzzy |42
235257
(1 row)
236258

237259
update trigtest set a = 2 where a = 1 returning *;
238-
a |b
239-
---+----
240-
1 | 42
260+
a | c | d
261+
---+-------+----
262+
1 |xyzzy |42
241263
(1 row)
242264

243265
select * from trigtest;
244-
a |b
245-
---+----
246-
1 | 42
266+
a | c | d
267+
---+-------+----
268+
1 |xyzzy |42
247269
(1 row)
248270

249271
drop table trigtest;

‎src/test/regress/sql/triggers.sql

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,10 @@ select * from trigtest;
155155
droptable trigtest;
156156

157157
-- Check behavior with an implicit column default, too (bug #16644)
158-
createtabletrigtest (ainteger);
158+
createtabletrigtest (
159+
ainteger,
160+
b bool default truenot null,
161+
ctext default'xyzzy'not null);
159162

160163
createtriggertrigger_return_old
161164
before insertordeleteorupdateon trigtest
@@ -164,7 +167,13 @@ create trigger trigger_return_old
164167
insert into trigtestvalues(1);
165168
select*from trigtest;
166169

167-
altertable trigtest add column binteger default42not null;
170+
altertable trigtest add column dinteger default42not null;
171+
172+
select*from trigtest;
173+
update trigtestset a=2where a=1 returning*;
174+
select*from trigtest;
175+
176+
altertable trigtest drop column b;
168177

169178
select*from trigtest;
170179
update trigtestset a=2where a=1 returning*;

0 commit comments

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