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

Commit7aeb640

Browse files
committed
In INSERT/UPDATE, use the table's real tuple descriptor as target.
This back-patches commit 20d3fe900 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 parent5ca6f68 commit7aeb640

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
@@ -88,8 +88,6 @@ static bool GetTupleForTrigger(EState *estate,
8888
LockTupleModelockmode,
8989
TupleTableSlot*oldslot,
9090
TupleTableSlot**newSlot);
91-
staticHeapTupleMaterializeTupleForTrigger(TupleTableSlot*slot,
92-
bool*shouldFree);
9391
staticboolTriggerEnabled(EState*estate,ResultRelInfo*relinfo,
9492
Trigger*trigger,TriggerEventevent,
9593
Bitmapset*modifiedCols,
@@ -2673,7 +2671,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
26732671
ExecCopySlot(newslot,epqslot_clean);
26742672
}
26752673

2676-
trigtuple=MaterializeTupleForTrigger(oldslot,&should_free_trig);
2674+
trigtuple=ExecFetchSlotHeapTuple(oldslot, true,&should_free_trig);
26772675
}
26782676
else
26792677
{
@@ -3042,40 +3040,6 @@ GetTupleForTrigger(EState *estate,
30423040
return true;
30433041
}
30443042

3045-
/*
3046-
* Extract a HeapTuple that we can pass off to trigger functions.
3047-
*
3048-
* We must materialize the tuple and make sure it is not dependent on any
3049-
* attrmissing data. This is needed for the old row in BEFORE UPDATE
3050-
* triggers, since they can choose to pass back this exact tuple as the update
3051-
* result, causing the tuple to be inserted into an executor slot that lacks
3052-
* the attrmissing data.
3053-
*
3054-
* Currently we don't seem to need to remove the attrmissing dependency in any
3055-
* other cases, but keep this as a separate function to simplify fixing things
3056-
* if that changes.
3057-
*/
3058-
staticHeapTuple
3059-
MaterializeTupleForTrigger(TupleTableSlot*slot,bool*shouldFree)
3060-
{
3061-
HeapTupletup;
3062-
TupleDesctupdesc=slot->tts_tupleDescriptor;
3063-
3064-
tup=ExecFetchSlotHeapTuple(slot, true,shouldFree);
3065-
if (HeapTupleHeaderGetNatts(tup->t_data)<tupdesc->natts&&
3066-
tupdesc->constr&&tupdesc->constr->missing)
3067-
{
3068-
HeapTuplenewtup;
3069-
3070-
newtup=heap_expand_tuple(tup,tupdesc);
3071-
if (*shouldFree)
3072-
heap_freetuple(tup);
3073-
*shouldFree= true;
3074-
tup=newtup;
3075-
}
3076-
returntup;
3077-
}
3078-
30793043
/*
30803044
* Is trigger enabled to fire?
30813045
*/

‎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
@@ -2681,15 +2681,27 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
26812681
TupleTableSlot*junkresslot;
26822682

26832683
subplan=mtstate->mt_plans[i]->plan;
2684-
if (operation==CMD_INSERT||operation==CMD_UPDATE)
2685-
ExecCheckPlanOutput(resultRelInfo->ri_RelationDesc,
2686-
subplan->targetlist);
26872684

26882685
junkresslot=
26892686
ExecInitExtraTupleSlot(estate,NULL,
26902687
table_slot_callbacks(resultRelInfo->ri_RelationDesc));
2691-
j=ExecInitJunkFilter(subplan->targetlist,
2692-
junkresslot);
2688+
2689+
/*
2690+
* For an INSERT or UPDATE, the result tuple must always match
2691+
* the target table's descriptor. For a DELETE, it won't
2692+
* (indeed, there's probably no non-junk output columns).
2693+
*/
2694+
if (operation==CMD_INSERT||operation==CMD_UPDATE)
2695+
{
2696+
ExecCheckPlanOutput(resultRelInfo->ri_RelationDesc,
2697+
subplan->targetlist);
2698+
j=ExecInitJunkFilterInsertion(subplan->targetlist,
2699+
RelationGetDescr(resultRelInfo->ri_RelationDesc),
2700+
junkresslot);
2701+
}
2702+
else
2703+
j=ExecInitJunkFilter(subplan->targetlist,
2704+
junkresslot);
26932705

26942706
if (operation==CMD_UPDATE||operation==CMD_DELETE)
26952707
{

‎src/include/executor/executor.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,9 @@ extern void ResetTupleHashTable(TupleHashTable hashtable);
156156
*/
157157
externJunkFilter*ExecInitJunkFilter(List*targetList,
158158
TupleTableSlot*slot);
159+
externJunkFilter*ExecInitJunkFilterInsertion(List*targetList,
160+
TupleDesccleanTupType,
161+
TupleTableSlot*slot);
159162
externJunkFilter*ExecInitJunkFilterConversion(List*targetList,
160163
TupleDesccleanTupType,
161164
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