diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index d4427917476..3d8333bb848 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -88,8 +88,6 @@ static bool GetTupleForTrigger(EState *estate, LockTupleMode lockmode, TupleTableSlot *oldslot, TupleTableSlot **newSlot); -static HeapTuple MaterializeTupleForTrigger(TupleTableSlot *slot, - bool *shouldFree); static bool TriggerEnabled(EState *estate, ResultRelInfo *relinfo, Trigger *trigger, TriggerEvent event, Bitmapset *modifiedCols, @@ -2673,7 +2671,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, ExecCopySlot(newslot, epqslot_clean); } - trigtuple = MaterializeTupleForTrigger(oldslot, &should_free_trig); + trigtuple = ExecFetchSlotHeapTuple(oldslot, true, &should_free_trig); } else { @@ -3042,40 +3040,6 @@ GetTupleForTrigger(EState *estate, return true; } -/* - * Extract a HeapTuple that we can pass off to trigger functions. - * - * We must materialize the tuple and make sure it is not dependent on any - * attrmissing data. This is needed for the old row in BEFORE UPDATE - * triggers, since they can choose to pass back this exact tuple as the update - * result, causing the tuple to be inserted into an executor slot that lacks - * the attrmissing data. - * - * Currently we don't seem to need to remove the attrmissing dependency in any - * other cases, but keep this as a separate function to simplify fixing things - * if that changes. - */ -static HeapTuple -MaterializeTupleForTrigger(TupleTableSlot *slot, bool *shouldFree) -{ - HeapTuple tup; - TupleDesc tupdesc = slot->tts_tupleDescriptor; - - tup = ExecFetchSlotHeapTuple(slot, true, shouldFree); - if (HeapTupleHeaderGetNatts(tup->t_data) < tupdesc->natts && - tupdesc->constr && tupdesc->constr->missing) - { - HeapTuple newtup; - - newtup = heap_expand_tuple(tup, tupdesc); - if (*shouldFree) - heap_freetuple(tup); - *shouldFree = true; - tup = newtup; - } - return tup; -} - /* * Is trigger enabled to fire? */ diff --git a/src/backend/executor/execJunk.c b/src/backend/executor/execJunk.c index 40d700dd9e2..1a822ff24b3 100644 --- a/src/backend/executor/execJunk.c +++ b/src/backend/executor/execJunk.c @@ -54,23 +54,48 @@ * * The source targetlist is passed in. The output tuple descriptor is * built from the non-junk tlist entries. - * An optional resultSlot can be passed as well. + * An optional resultSlot can be passed as well; otherwise, we create one. */ JunkFilter * ExecInitJunkFilter(List *targetList, TupleTableSlot *slot) { - JunkFilter *junkfilter; TupleDesc cleanTupType; - int cleanLength; - AttrNumber *cleanMap; - ListCell *t; - AttrNumber cleanResno; /* * Compute the tuple descriptor for the cleaned tuple. */ cleanTupType = ExecCleanTypeFromTL(targetList); + /* + * The rest is the same as ExecInitJunkFilterInsertion, ie, we want to map + * every non-junk targetlist column into the output tuple. + */ + return ExecInitJunkFilterInsertion(targetList, cleanTupType, slot); +} + +/* + * ExecInitJunkFilterInsertion + * + * Initialize a JunkFilter for insertions into a table. + * + * Here, we are given the target "clean" tuple descriptor rather than + * inferring it from the targetlist. Although the target descriptor can + * contain deleted columns, that is not of concern here, since the targetlist + * should contain corresponding NULL constants (cf. ExecCheckPlanOutput). + * It is assumed that the caller has checked that the table's columns match up + * with the non-junk columns of the targetlist. + */ +JunkFilter * +ExecInitJunkFilterInsertion(List *targetList, + TupleDesc cleanTupType, + TupleTableSlot *slot) +{ + JunkFilter *junkfilter; + int cleanLength; + AttrNumber *cleanMap; + ListCell *t; + AttrNumber cleanResno; + /* * Use the given slot, or make a new slot if we weren't given one. */ @@ -93,17 +118,18 @@ ExecInitJunkFilter(List *targetList, TupleTableSlot *slot) if (cleanLength > 0) { cleanMap = (AttrNumber *) palloc(cleanLength * sizeof(AttrNumber)); - cleanResno = 1; + cleanResno = 0; foreach(t, targetList) { TargetEntry *tle = lfirst(t); if (!tle->resjunk) { - cleanMap[cleanResno - 1] = tle->resno; + cleanMap[cleanResno] = tle->resno; cleanResno++; } } + Assert(cleanResno == cleanLength); } else cleanMap = NULL; diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 20a4c474cc4..f450e4d80ad 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -2681,15 +2681,27 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) TupleTableSlot *junkresslot; subplan = mtstate->mt_plans[i]->plan; - if (operation == CMD_INSERT || operation == CMD_UPDATE) - ExecCheckPlanOutput(resultRelInfo->ri_RelationDesc, - subplan->targetlist); junkresslot = ExecInitExtraTupleSlot(estate, NULL, table_slot_callbacks(resultRelInfo->ri_RelationDesc)); - j = ExecInitJunkFilter(subplan->targetlist, - junkresslot); + + /* + * For an INSERT or UPDATE, the result tuple must always match + * the target table's descriptor. For a DELETE, it won't + * (indeed, there's probably no non-junk output columns). + */ + if (operation == CMD_INSERT || operation == CMD_UPDATE) + { + ExecCheckPlanOutput(resultRelInfo->ri_RelationDesc, + subplan->targetlist); + j = ExecInitJunkFilterInsertion(subplan->targetlist, + RelationGetDescr(resultRelInfo->ri_RelationDesc), + junkresslot); + } + else + j = ExecInitJunkFilter(subplan->targetlist, + junkresslot); if (operation == CMD_UPDATE || operation == CMD_DELETE) { diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 415e117407c..238b77494f4 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -156,6 +156,9 @@ extern void ResetTupleHashTable(TupleHashTable hashtable); */ extern JunkFilter *ExecInitJunkFilter(List *targetList, TupleTableSlot *slot); +extern JunkFilter *ExecInitJunkFilterInsertion(List *targetList, + TupleDesc cleanTupType, + TupleTableSlot *slot); extern JunkFilter *ExecInitJunkFilterConversion(List *targetList, TupleDesc cleanTupType, TupleTableSlot *slot); diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index c19aac96742..f6610b0585b 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -216,34 +216,56 @@ select * from trigtest; drop table trigtest; -- Check behavior with an implicit column default, too (bug #16644) -create table trigtest (a integer); +create table trigtest ( + a integer, + b bool default true not null, + c text default 'xyzzy' not null); create trigger trigger_return_old before insert or delete or update on trigtest for each row execute procedure trigger_return_old(); insert into trigtest values(1); select * from trigtest; - a ---- - 1 + a | b | c +---+---+------- + 1 | t | xyzzy (1 row) -alter table trigtest add column b integer default 42 not null; +alter table trigtest add column d integer default 42 not null; select * from trigtest; - a | b ----+---- - 1 | 42 + a | b | c | d +---+---+-------+---- + 1 | t | xyzzy | 42 (1 row) update trigtest set a = 2 where a = 1 returning *; - a | b ----+---- - 1 | 42 + a | b | c | d +---+---+-------+---- + 1 | t | xyzzy | 42 (1 row) select * from trigtest; - a | b ----+---- - 1 | 42 + a | b | c | d +---+---+-------+---- + 1 | t | xyzzy | 42 +(1 row) + +alter table trigtest drop column b; +select * from trigtest; + a | c | d +---+-------+---- + 1 | xyzzy | 42 +(1 row) + +update trigtest set a = 2 where a = 1 returning *; + a | c | d +---+-------+---- + 1 | xyzzy | 42 +(1 row) + +select * from trigtest; + a | c | d +---+-------+---- + 1 | xyzzy | 42 (1 row) drop table trigtest; diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index bf2e73abf60..9c3872eb145 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -155,7 +155,10 @@ select * from trigtest; drop table trigtest; -- Check behavior with an implicit column default, too (bug #16644) -create table trigtest (a integer); +create table trigtest ( + a integer, + b bool default true not null, + c text default 'xyzzy' not null); create trigger trigger_return_old before insert or delete or update on trigtest @@ -164,7 +167,13 @@ create trigger trigger_return_old insert into trigtest values(1); select * from trigtest; -alter table trigtest add column b integer default 42 not null; +alter table trigtest add column d integer default 42 not null; + +select * from trigtest; +update trigtest set a = 2 where a = 1 returning *; +select * from trigtest; + +alter table trigtest drop column b; select * from trigtest; update trigtest set a = 2 where a = 1 returning *;