postgres.git / summary / log / commit / refs
commit 20d3fe9009ddbbbb3da3a2da298f922054b43f8c
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon Oct 26 15:36:53 2020 +0000
First released in: 14.0
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
src/backend/commands/trigger.c | 38 +-----------------------------
src/backend/executor/execJunk.c | 42 +++++++++++++++++++++++++++-------
src/backend/executor/nodeModifyTable.c | 22 ++++++++++++++----
src/include/executor/executor.h | 3 +++
4 files changed, 55 insertions(+), 50 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
{
@@ -3043,40 +3041,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 a33423c896e..29e07b72287 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -2591,15 +2591,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 b7978cd22eb..0c48d2a519e 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);
[parent: fa42c2ecb0f6]