In INSERT/UPDATE, use the table's real tuple descriptor as target.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 26 Oct 2020 15:36:53 +0000 (11:36 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 26 Oct 2020 15:36:53 +0000 (11:36 -0400)
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
src/backend/executor/execJunk.c
src/backend/executor/nodeModifyTable.c
src/include/executor/executor.h

index 59289f8d4d3702f046ba6811a62078becf67d89d..092ac1646de4074801f0bee66ba0869bdd5deb01 100644 (file)
@@ -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?
  */
index 40d700dd9e23ff3855b203f6220f55ba5764e463..1a822ff24b3829885aeb7cb48c3b2bea9ed7305b 100644 (file)
  *
  * 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;
index a33423c896e3703c982bc8ee0b90351054fee1b4..29e07b72287515c04d4f3192962571b7d8695765 100644 (file)
@@ -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)
                {
index b7978cd22ebc6fad72a36a56b0cbc3e4bea12f0c..0c48d2a519e835079b093f3781e86dc2ad8e3cda 100644 (file)
@@ -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);