From ba9f18abd3650e385e9a35df7145a7c38af17e92 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 25 Oct 2020 13:57:46 -0400 Subject: Fix corner case for a BEFORE ROW UPDATE trigger returning OLD. If the old row has any "missing" attributes that are supposed to be retrieved from an associated tuple descriptor, the wrong things happened because the trigger result is shoved directly into an executor slot that lacks the missing-attribute data. Notably, CHECK-constraint verification would incorrectly see those columns as NULL, and so would RETURNING-list evaluation. Band-aid around this by forcibly expanding the tuple before passing it to the trigger function. (IMO it was a fundamental misdesign to put the missing-attribute data into tuple constraints, which so much of the system considers to be optional. But we're probably stuck with that now, and will have to continue to apply band-aids as we find other places with similar issues.) Back-patch to v12. v11 would also have the issue, except that commit 920311ab1 already applied a similar band-aid. That forced expansion in more cases than seem really necessary, though, so this isn't a directly equivalent fix. Amit Langote, with some cosmetic changes by me Discussion: https://postgr.es/m/16644-5da7ef98a7ac4545@postgresql.org --- src/backend/commands/trigger.c | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) (limited to 'src/backend/commands/trigger.c') diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 28b98d10ae8..59289f8d4d3 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -89,6 +89,8 @@ 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, @@ -2672,7 +2674,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, ExecCopySlot(newslot, epqslot_clean); } - trigtuple = ExecFetchSlotHeapTuple(oldslot, true, &should_free_trig); + trigtuple = MaterializeTupleForTrigger(oldslot, &should_free_trig); } else { @@ -2925,6 +2927,9 @@ ExecASTruncateTriggers(EState *estate, ResultRelInfo *relinfo) } +/* + * Fetch tuple into "oldslot", dealing with locking and EPQ if necessary + */ static bool GetTupleForTrigger(EState *estate, EPQState *epqstate, @@ -3038,6 +3043,40 @@ 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? */ -- cgit v1.2.3