Revise child-to-root tuple conversion map management.
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Mon, 19 Oct 2020 11:11:54 +0000 (14:11 +0300)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Mon, 19 Oct 2020 11:42:55 +0000 (14:42 +0300)
Store the tuple conversion map to convert a tuple from a child table's
format to the root format in a new ri_ChildToRootMap field in
ResultRelInfo. It is initialized if transition tuple capture for FOR
STATEMENT triggers or INSERT tuple routing on a partitioned table is
needed. Previously, ModifyTable kept the maps in the per-subplan
ModifyTableState->mt_per_subplan_tupconv_maps array, or when tuple
routing was used, in
ResultRelInfo->ri_Partitioninfo->pi_PartitionToRootMap. The new field
replaces both of those.

Now that the child-to-root tuple conversion map is always available in
ResultRelInfo (when needed), remove the TransitionCaptureState.tcs_map
field. The callers of Exec*Trigger() functions no longer need to set or
save it, which is much less confusing and bug-prone. Also, as a future
optimization, this will allow us to delay creating the map for a given
result relation until the relation is actually processed during
execution.

Author: Amit Langote
Discussion: https://www.postgresql.org/message-id/CA%2BHiwqHtCWLdK-LO%3DNEsvOdHx%2B7yv4mE_zYK0i3BH7dXb-wxog%40mail.gmail.com

src/backend/commands/copy.c
src/backend/commands/trigger.c
src/backend/executor/execMain.c
src/backend/executor/execPartition.c
src/backend/executor/nodeModifyTable.c
src/include/commands/trigger.h
src/include/executor/execPartition.h
src/include/nodes/execnodes.h

index 531bd7c73a52d6b6bdcebac943649c18641722c8..b4ccd35ec1809f94c1b056ae196d0f4ea5debc2b 100644 (file)
@@ -3106,31 +3106,14 @@ CopyFrom(CopyState cstate)
 
                        /*
                         * If we're capturing transition tuples, we might need to convert
-                        * from the partition rowtype to root rowtype.
+                        * from the partition rowtype to root rowtype. But if there are no
+                        * BEFORE triggers on the partition that could change the tuple,
+                        * we can just remember the original unconverted tuple to avoid a
+                        * needless round trip conversion.
                         */
                        if (cstate->transition_capture != NULL)
-                       {
-                               if (has_before_insert_row_trig)
-                               {
-                                       /*
-                                        * If there are any BEFORE triggers on the partition,
-                                        * we'll have to be ready to convert their result back to
-                                        * tuplestore format.
-                                        */
-                                       cstate->transition_capture->tcs_original_insert_tuple = NULL;
-                                       cstate->transition_capture->tcs_map =
-                                               resultRelInfo->ri_PartitionInfo->pi_PartitionToRootMap;
-                               }
-                               else
-                               {
-                                       /*
-                                        * Otherwise, just remember the original unconverted
-                                        * tuple, to avoid a needless round trip conversion.
-                                        */
-                                       cstate->transition_capture->tcs_original_insert_tuple = myslot;
-                                       cstate->transition_capture->tcs_map = NULL;
-                               }
-                       }
+                               cstate->transition_capture->tcs_original_insert_tuple =
+                                       !has_before_insert_row_trig ? myslot : NULL;
 
                        /*
                         * We might need to convert from the root rowtype to the partition
index 3b4fbdadf4f3138db7e1b8979020d3249483b05c..5719672f4ce15f92e536e42bacf7c74fe3519d11 100644 (file)
@@ -35,6 +35,7 @@
 #include "commands/defrem.h"
 #include "commands/trigger.h"
 #include "executor/executor.h"
+#include "executor/execPartition.h"
 #include "miscadmin.h"
 #include "nodes/bitmapset.h"
 #include "nodes/makefuncs.h"
@@ -4292,9 +4293,10 @@ GetAfterTriggersTableData(Oid relid, CmdType cmdType)
  * If there are no triggers in 'trigdesc' that request relevant transition
  * tables, then return NULL.
  *
- * The resulting object can be passed to the ExecAR* functions.  The caller
- * should set tcs_map or tcs_original_insert_tuple as appropriate when dealing
- * with child tables.
+ * The resulting object can be passed to the ExecAR* functions.  When
+ * dealing with child tables, the caller can set tcs_original_insert_tuple
+ * to avoid having to reconstruct the original tuple in the root table's
+ * format.
  *
  * Note that we copy the flags from a parent table into this struct (rather
  * than subsequently using the relation's TriggerDesc directly) so that we can
@@ -5389,7 +5391,7 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
        if (row_trigger && transition_capture != NULL)
        {
                TupleTableSlot *original_insert_tuple = transition_capture->tcs_original_insert_tuple;
-               TupleConversionMap *map = transition_capture->tcs_map;
+               TupleConversionMap *map = relinfo->ri_ChildToRootMap;
                bool            delete_old_table = transition_capture->tcs_delete_old_table;
                bool            update_old_table = transition_capture->tcs_update_old_table;
                bool            update_new_table = transition_capture->tcs_update_new_table;
index 293f53d07c9d1f562e05c1c472b3dec448b89db0..fcdd4b356741f8122fd86b79c46087c0c41c1d9b 100644 (file)
@@ -1244,6 +1244,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
        resultRelInfo->ri_TrigNewSlot = NULL;
        resultRelInfo->ri_PartitionRoot = partition_root;
        resultRelInfo->ri_PartitionInfo = NULL; /* may be set later */
+       resultRelInfo->ri_ChildToRootMap = NULL;
        resultRelInfo->ri_CopyMultiInsertBuffer = NULL;
 }
 
index 33d2c6f63de4a15b9d1d60f7f49c914e58d155fe..08f91e59a7acd95cd4b2d4e2808b62b65d5c08c9 100644 (file)
@@ -907,6 +907,15 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
                }
        }
 
+       /*
+        * Also, if transition capture is required, store a map to convert tuples
+        * from partition's rowtype to the root partition table's.
+        */
+       if (mtstate->mt_transition_capture || mtstate->mt_oc_transition_capture)
+               leaf_part_rri->ri_ChildToRootMap =
+                       convert_tuples_by_name(RelationGetDescr(leaf_part_rri->ri_RelationDesc),
+                                                                  RelationGetDescr(leaf_part_rri->ri_PartitionRoot));
+
        /*
         * Since we've just initialized this ResultRelInfo, it's not in any list
         * attached to the estate as yet.  Add it, so that it can be found later.
@@ -976,20 +985,6 @@ ExecInitRoutingInfo(ModifyTableState *mtstate,
        else
                partrouteinfo->pi_PartitionTupleSlot = NULL;
 
-       /*
-        * Also, if transition capture is required, store a map to convert tuples
-        * from partition's rowtype to the root partition table's.
-        */
-       if (mtstate &&
-               (mtstate->mt_transition_capture || mtstate->mt_oc_transition_capture))
-       {
-               partrouteinfo->pi_PartitionToRootMap =
-                       convert_tuples_by_name(RelationGetDescr(partRelInfo->ri_RelationDesc),
-                                                                  RelationGetDescr(partRelInfo->ri_PartitionRoot));
-       }
-       else
-               partrouteinfo->pi_PartitionToRootMap = NULL;
-
        /*
         * If the partition is a foreign table, let the FDW init itself for
         * routing tuples to the partition.
index ae209bc774a316546cb8e4fe685198e81d0de968..5f85fd7cd88fd9a3b49382326928fd6646650cae 100644 (file)
@@ -72,9 +72,6 @@ static TupleTableSlot *ExecPrepareTupleRouting(ModifyTableState *mtstate,
                                                                                           ResultRelInfo *targetRelInfo,
                                                                                           TupleTableSlot *slot,
                                                                                           ResultRelInfo **partRelInfo);
-static void ExecSetupChildParentMapForSubplan(ModifyTableState *mtstate);
-static TupleConversionMap *tupconv_map_for_subplan(ModifyTableState *node,
-                                                                                                  int whichplan);
 
 /*
  * Verify that the tuples to be produced by INSERT or UPDATE match the
@@ -1086,9 +1083,7 @@ ExecCrossPartitionUpdate(ModifyTableState *mtstate,
 {
        EState     *estate = mtstate->ps.state;
        PartitionTupleRouting *proute = mtstate->mt_partition_tuple_routing;
-       int                     map_index;
        TupleConversionMap *tupconv_map;
-       TupleConversionMap *saved_tcs_map = NULL;
        bool            tuple_deleted;
        TupleTableSlot *epqslot = NULL;
 
@@ -1163,37 +1158,25 @@ ExecCrossPartitionUpdate(ModifyTableState *mtstate,
 
        /*
         * resultRelInfo is one of the per-subplan resultRelInfos.  So we should
-        * convert the tuple into root's tuple descriptor, since ExecInsert()
-        * starts the search from root.  The tuple conversion map list is in the
-        * order of mtstate->resultRelInfo[], so to retrieve the one for this
-        * resultRel, we need to know the position of the resultRel in
-        * mtstate->resultRelInfo[].
+        * convert the tuple into root's tuple descriptor if needed, since
+        * ExecInsert() starts the search from root.
         */
-       map_index = resultRelInfo - mtstate->resultRelInfo;
-       Assert(map_index >= 0 && map_index < mtstate->mt_nplans);
-       tupconv_map = tupconv_map_for_subplan(mtstate, map_index);
+       tupconv_map = resultRelInfo->ri_ChildToRootMap;
        if (tupconv_map != NULL)
                slot = execute_attr_map_slot(tupconv_map->attrMap,
                                                                         slot,
                                                                         mtstate->mt_root_tuple_slot);
 
-       /*
-        * ExecInsert() may scribble on mtstate->mt_transition_capture, so save
-        * the currently active map.
-        */
-       if (mtstate->mt_transition_capture)
-               saved_tcs_map = mtstate->mt_transition_capture->tcs_map;
-
        /* Tuple routing starts from the root table. */
        *inserted_tuple = ExecInsert(mtstate, mtstate->rootResultRelInfo, slot,
                                                                 planSlot, estate, canSetTag);
 
-       /* Clear the INSERT's tuple and restore the saved map. */
+       /*
+        * Reset the transition state that may possibly have been written
+        * by INSERT.
+        */
        if (mtstate->mt_transition_capture)
-       {
                mtstate->mt_transition_capture->tcs_original_insert_tuple = NULL;
-               mtstate->mt_transition_capture->tcs_map = saved_tcs_map;
-       }
 
        /* We're done moving. */
        return true;
@@ -1870,28 +1853,6 @@ ExecSetupTransitionCaptureState(ModifyTableState *mtstate, EState *estate)
                        MakeTransitionCaptureState(targetRelInfo->ri_TrigDesc,
                                                                           RelationGetRelid(targetRelInfo->ri_RelationDesc),
                                                                           CMD_UPDATE);
-
-       /*
-        * If we found that we need to collect transition tuples then we may also
-        * need tuple conversion maps for any children that have TupleDescs that
-        * aren't compatible with the tuplestores.  (We can share these maps
-        * between the regular and ON CONFLICT cases.)
-        */
-       if (mtstate->mt_transition_capture != NULL ||
-               mtstate->mt_oc_transition_capture != NULL)
-       {
-               ExecSetupChildParentMapForSubplan(mtstate);
-
-               /*
-                * Install the conversion map for the first plan for UPDATE and DELETE
-                * operations.  It will be advanced each time we switch to the next
-                * plan.  (INSERT operations set it every time, so we need not update
-                * mtstate->mt_oc_transition_capture here.)
-                */
-               if (mtstate->mt_transition_capture && mtstate->operation != CMD_INSERT)
-                       mtstate->mt_transition_capture->tcs_map =
-                               tupconv_map_for_subplan(mtstate, 0);
-       }
 }
 
 /*
@@ -1929,35 +1890,20 @@ ExecPrepareTupleRouting(ModifyTableState *mtstate,
 
        /*
         * If we're capturing transition tuples, we might need to convert from the
-        * partition rowtype to root partitioned table's rowtype.
+        * partition rowtype to root partitioned table's rowtype.  But if there
+        * are no BEFORE triggers on the partition that could change the tuple, we
+        * can just remember the original unconverted tuple to avoid a needless
+        * round trip conversion.
         */
        if (mtstate->mt_transition_capture != NULL)
        {
-               if (partrel->ri_TrigDesc &&
-                       partrel->ri_TrigDesc->trig_insert_before_row)
-               {
-                       /*
-                        * If there are any BEFORE triggers on the partition, we'll have
-                        * to be ready to convert their result back to tuplestore format.
-                        */
-                       mtstate->mt_transition_capture->tcs_original_insert_tuple = NULL;
-                       mtstate->mt_transition_capture->tcs_map =
-                               partrouteinfo->pi_PartitionToRootMap;
-               }
-               else
-               {
-                       /*
-                        * Otherwise, just remember the original unconverted tuple, to
-                        * avoid a needless round trip conversion.
-                        */
-                       mtstate->mt_transition_capture->tcs_original_insert_tuple = slot;
-                       mtstate->mt_transition_capture->tcs_map = NULL;
-               }
-       }
-       if (mtstate->mt_oc_transition_capture != NULL)
-       {
-               mtstate->mt_oc_transition_capture->tcs_map =
-                       partrouteinfo->pi_PartitionToRootMap;
+               bool            has_before_insert_row_trig;
+
+               has_before_insert_row_trig = (partrel->ri_TrigDesc &&
+                                                                         partrel->ri_TrigDesc->trig_insert_before_row);
+
+               mtstate->mt_transition_capture->tcs_original_insert_tuple =
+                       !has_before_insert_row_trig ? slot : NULL;
        }
 
        /*
@@ -1975,58 +1921,6 @@ ExecPrepareTupleRouting(ModifyTableState *mtstate,
        return slot;
 }
 
-/*
- * Initialize the child-to-root tuple conversion map array for UPDATE subplans.
- *
- * This map array is required to convert the tuple from the subplan result rel
- * to the target table descriptor. This requirement arises for two independent
- * scenarios:
- * 1. For update-tuple-routing.
- * 2. For capturing tuples in transition tables.
- */
-static void
-ExecSetupChildParentMapForSubplan(ModifyTableState *mtstate)
-{
-       ResultRelInfo *targetRelInfo = mtstate->rootResultRelInfo;
-       ResultRelInfo *resultRelInfos = mtstate->resultRelInfo;
-       TupleDesc       outdesc;
-       int                     numResultRelInfos = mtstate->mt_nplans;
-       int                     i;
-
-       /*
-        * Build array of conversion maps from each child's TupleDesc to the one
-        * used in the target relation.  The map pointers may be NULL when no
-        * conversion is necessary, which is hopefully a common case.
-        */
-
-       /* Get tuple descriptor of the target rel. */
-       outdesc = RelationGetDescr(targetRelInfo->ri_RelationDesc);
-
-       mtstate->mt_per_subplan_tupconv_maps = (TupleConversionMap **)
-               palloc(sizeof(TupleConversionMap *) * numResultRelInfos);
-
-       for (i = 0; i < numResultRelInfos; ++i)
-       {
-               mtstate->mt_per_subplan_tupconv_maps[i] =
-                       convert_tuples_by_name(RelationGetDescr(resultRelInfos[i].ri_RelationDesc),
-                                                                  outdesc);
-       }
-}
-
-/*
- * For a given subplan index, get the tuple conversion map.
- */
-static TupleConversionMap *
-tupconv_map_for_subplan(ModifyTableState *mtstate, int whichplan)
-{
-       /* If nobody else set the per-subplan array of maps, do so ourselves. */
-       if (mtstate->mt_per_subplan_tupconv_maps == NULL)
-               ExecSetupChildParentMapForSubplan(mtstate);
-
-       Assert(whichplan >= 0 && whichplan < mtstate->mt_nplans);
-       return mtstate->mt_per_subplan_tupconv_maps[whichplan];
-}
-
 /* ----------------------------------------------------------------
  *        ExecModifyTable
  *
@@ -2122,17 +2016,6 @@ ExecModifyTable(PlanState *pstate)
                                junkfilter = resultRelInfo->ri_junkFilter;
                                EvalPlanQualSetPlan(&node->mt_epqstate, subplanstate->plan,
                                                                        node->mt_arowmarks[node->mt_whichplan]);
-                               /* Prepare to convert transition tuples from this child. */
-                               if (node->mt_transition_capture != NULL)
-                               {
-                                       node->mt_transition_capture->tcs_map =
-                                               tupconv_map_for_subplan(node, node->mt_whichplan);
-                               }
-                               if (node->mt_oc_transition_capture != NULL)
-                               {
-                                       node->mt_oc_transition_capture->tcs_map =
-                                               tupconv_map_for_subplan(node, node->mt_whichplan);
-                               }
                                continue;
                        }
                        else
@@ -2334,8 +2217,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
         * If it's a partitioned table, the root partition doesn't appear
         * elsewhere in the plan and its RT index is given explicitly in
         * node->rootRelation.  Otherwise (i.e. table inheritance) the target
-        * relation is the first relation in the node->resultRelations list, and
-        * we will initialize it in the loop below.
+        * relation is the first relation in the node->resultRelations list.
         *----------
         */
        if (node->rootRelation > 0)
@@ -2347,6 +2229,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
        else
        {
                mtstate->rootResultRelInfo = mtstate->resultRelInfo;
+               ExecInitResultRelation(estate, mtstate->resultRelInfo,
+                                                          linitial_int(node->resultRelations));
        }
 
        mtstate->mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans);
@@ -2356,6 +2240,13 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
        EvalPlanQualInit(&mtstate->mt_epqstate, estate, NULL, NIL, node->epqParam);
        mtstate->fireBSTriggers = true;
 
+       /*
+        * Build state for collecting transition tuples.  This requires having a
+        * valid trigger query context, so skip it in explain-only mode.
+        */
+       if (!(eflags & EXEC_FLAG_EXPLAIN_ONLY))
+               ExecSetupTransitionCaptureState(mtstate, estate);
+
        /*
         * call ExecInitNode on each of the plans to be executed and save the
         * results into the array "mt_plans".  This is also a convenient place to
@@ -2370,8 +2261,12 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 
                subplan = (Plan *) lfirst(l1);
 
-               /* This opens the relation and fills ResultRelInfo. */
-               ExecInitResultRelation(estate, resultRelInfo, resultRelation);
+               /*
+                * This opens result relation and fills ResultRelInfo. (root relation
+                * was initialized already.)
+                */
+               if (resultRelInfo != mtstate->rootResultRelInfo)
+                       ExecInitResultRelation(estate, resultRelInfo, resultRelation);
 
                /* Initialize the usesFdwDirectModify flag */
                resultRelInfo->ri_usesFdwDirectModify = bms_is_member(i,
@@ -2427,6 +2322,23 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
                                                                                                                         eflags);
                }
 
+               /*
+                * If needed, initialize a map to convert tuples in the child format
+                * to the format of the table mentioned in the query (root relation).
+                * It's needed for update tuple routing, because the routing starts
+                * from the root relation.  It's also needed for capturing transition
+                * tuples, because the transition tuple store can only store tuples
+                * in the root table format.
+                *
+                * For INSERT, the map is only initialized for a given partition when
+                * the partition itself is first initialized by ExecFindPartition().
+                */
+               if (update_tuple_routing_needed ||
+                       (mtstate->mt_transition_capture &&
+                        mtstate->operation != CMD_INSERT))
+                       resultRelInfo->ri_ChildToRootMap =
+                               convert_tuples_by_name(RelationGetDescr(resultRelInfo->ri_RelationDesc),
+                                                                          RelationGetDescr(mtstate->rootResultRelInfo->ri_RelationDesc));
                resultRelInfo++;
                i++;
        }
@@ -2451,26 +2363,12 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
                        ExecSetupPartitionTupleRouting(estate, mtstate, rel);
 
        /*
-        * Build state for collecting transition tuples.  This requires having a
-        * valid trigger query context, so skip it in explain-only mode.
-        */
-       if (!(eflags & EXEC_FLAG_EXPLAIN_ONLY))
-               ExecSetupTransitionCaptureState(mtstate, estate);
-
-       /*
-        * Construct mapping from each of the per-subplan partition attnos to the
-        * root attno.  This is required when during update row movement the tuple
-        * descriptor of a source partition does not match the root partitioned
-        * table descriptor.  In such a case we need to convert tuples to the root
-        * tuple descriptor, because the search for destination partition starts
-        * from the root.  We'll also need a slot to store these converted tuples.
-        * We can skip this setup if it's not a partition key update.
+        * For update row movement we'll need a dedicated slot to store the
+        * tuples that have been converted from partition format to the root
+        * table format.
         */
        if (update_tuple_routing_needed)
-       {
-               ExecSetupChildParentMapForSubplan(mtstate);
                mtstate->mt_root_tuple_slot = table_slot_create(rel, NULL);
-       }
 
        /*
         * Initialize any WITH CHECK OPTION constraints if needed.
index a40ddf5db52cab4771a3dfdd94703d11ca1e532b..e38d732ed477dd5670598994e74a11674191b66d 100644 (file)
@@ -46,7 +46,7 @@ typedef struct TriggerData
  * The state for capturing old and new tuples into transition tables for a
  * single ModifyTable node (or other operation source, e.g. copy.c).
  *
- * This is per-caller to avoid conflicts in setting tcs_map or
+ * This is per-caller to avoid conflicts in setting
  * tcs_original_insert_tuple.  Note, however, that the pointed-to
  * private data may be shared across multiple callers.
  */
@@ -65,14 +65,6 @@ typedef struct TransitionCaptureState
        bool            tcs_update_new_table;
        bool            tcs_insert_new_table;
 
-       /*
-        * For UPDATE and DELETE, AfterTriggerSaveEvent may need to convert the
-        * new and old tuples from a child table's format to the format of the
-        * relation named in a query so that it is compatible with the transition
-        * tuplestores.  The caller must store the conversion map here if so.
-        */
-       TupleConversionMap *tcs_map;
-
        /*
         * For INSERT and COPY, it would be wasteful to convert tuples from child
         * format to parent format after they have already been converted in the
index 6d1b72219872946abc59911f07d3f0438d52ffac..74c39911b2a2a0c3397042586b9ae4e6673bcb1a 100644 (file)
@@ -36,12 +36,6 @@ typedef struct PartitionRoutingInfo
         */
        TupleConversionMap *pi_RootToPartitionMap;
 
-       /*
-        * Map for converting tuples in partition format into the root partitioned
-        * table format, or NULL if no conversion is required.
-        */
-       TupleConversionMap *pi_PartitionToRootMap;
-
        /*
         * Slot to store tuples in partition format, or NULL when no translation
         * is required between root and partition.
index dff34fbc14e294e9fe4b48cee1dba85972774b41..46789cb00703ac1cbdc549cfeebd895b71708a75 100644 (file)
@@ -486,6 +486,13 @@ typedef struct ResultRelInfo
        /* info for partition tuple routing (NULL if not set up yet) */
        struct PartitionRoutingInfo *ri_PartitionInfo;
 
+       /*
+        * Map to convert child result relation tuples to the format of the table
+        * actually mentioned in the query (called "root").  Set only if
+        * transition tuple capture or update partition row movement is active.
+        */
+       TupleConversionMap *ri_ChildToRootMap;
+
        /* for use by copy.c when performing multi-inserts */
        struct CopyMultiInsertBuffer *ri_CopyMultiInsertBuffer;
 } ResultRelInfo;
@@ -1179,9 +1186,6 @@ typedef struct ModifyTableState
 
        /* controls transition table population for INSERT...ON CONFLICT UPDATE */
        struct TransitionCaptureState *mt_oc_transition_capture;
-
-       /* Per plan map for tuple conversion from child to root */
-       TupleConversionMap **mt_per_subplan_tupconv_maps;
 } ModifyTableState;
 
 /* ----------------