In INSERT/UPDATE, use the table's real tuple descriptor as target.

Previously, ExecInitModifyTable relied on ExecInitJunkFilter,
and thence ExecCleanTypeFromTL, to build the target descriptor from
the query tlist.  While we just checked (in ExecCheckPlanOutput)
that the tlist produces compatible output, this is not a great
substitute for the relation's actual tuple descriptor that's
available from the relcache.  For one thing, dropped columns will
not be correctly marked attisdropped; it's a bit surprising that
we've gotten away with that this long.  But the real reason for
being concerned with this is that using the table's descriptor means
that the slot will have correct attrmissing data, allowing us to
revert the klugy fix of commit ba9f18abd.  (This commit undoes
that one's changes in trigger.c, but keeps the new test case.)
Thus we can solve the bogus-trigger-tuple problem with fewer cycles
rather than more.

No back-patch, since this doesn't fix any additional bug, and it
seems somewhat more likely to have unforeseen side effects than
ba9f18abd's narrow fix.

Discussion: https://postgr.es/m/16644-5da7ef98a7ac4545@postgresql.org

1 files changed, 1 insertions, 37 deletions

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 59289f8d4d3..092ac1646de 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -89,8 +89,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,
@@ -2674,7 +2672,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 				ExecCopySlot(newslot, epqslot_clean);
 		}
 
-		trigtuple = MaterializeTupleForTrigger(oldslot, &should_free_trig);
+		trigtuple = ExecFetchSlotHeapTuple(oldslot, true, &should_free_trig);
 	}
 	else
 	{
@@ -3044,40 +3042,6 @@ GetTupleForTrigger(EState *estate,
 }
 
 /*
- * 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?
  */
 static bool