From f012daa2a2380582b5d56dfcb7810b363665846c Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 15 Dec 2016 17:39:19 +0900 Subject: [PATCH 5/7] Fix oddities of tuple-routing and TupleTableSlots We must use the partition's tuple descriptor *after* a tuple is routed, not the root table's. Partition's attributes, for example, may be ordered diferently from the root table's. We must then switch back to the root table's for the next tuple, because computing partition key of a tuple to be routed must be looking at the root table's tuple descriptor. A dedicated TupleTableSlot is allocated within EState called es_partition_tuple_slot whose descriptor is set to a given leaf partition for every input tuple after it's routed. Reported by: n/a Patch by: Amit Langote Reports: n/a --- src/backend/commands/copy.c | 28 +++++++++++++++++++++++++++- src/backend/executor/nodeModifyTable.c | 25 +++++++++++++++++++++++++ src/include/nodes/execnodes.h | 3 +++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index a0eb4241e2..bec8c73903 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -2441,6 +2441,15 @@ CopyFrom(CopyState cstate) estate->es_trig_tuple_slot = ExecInitExtraTupleSlot(estate); /* + * Initialize a dedicated slot to manipulate tuples of any given + * partition's rowtype. + */ + if (cstate->partition_dispatch_info) + estate->es_partition_tuple_slot = ExecInitExtraTupleSlot(estate); + else + estate->es_partition_tuple_slot = NULL; + + /* * It's more efficient to prepare a bunch of tuples for insertion, and * insert them in one heap_multi_insert() call, than call heap_insert() * separately for every tuple. However, we can't do that if there are @@ -2489,7 +2498,8 @@ CopyFrom(CopyState cstate) for (;;) { - TupleTableSlot *slot; + TupleTableSlot *slot, + *oldslot = NULL; bool skip_tuple; Oid loaded_oid = InvalidOid; @@ -2576,7 +2586,19 @@ CopyFrom(CopyState cstate) map = cstate->partition_tupconv_maps[leaf_part_index]; if (map) { + Relation partrel = resultRelInfo->ri_RelationDesc; + tuple = do_convert_tuple(tuple, map); + + /* + * We must use the partition's tuple descriptor from this + * point on. Use a dedicated slot from this point on until + * we're finished dealing with the partition. + */ + oldslot = slot; + slot = estate->es_partition_tuple_slot; + Assert(slot != NULL); + ExecSetSlotDescriptor(slot, RelationGetDescr(partrel)); ExecStoreTuple(tuple, slot, InvalidBuffer, true); } @@ -2672,6 +2694,10 @@ CopyFrom(CopyState cstate) { resultRelInfo = saved_resultRelInfo; estate->es_result_relation_info = resultRelInfo; + + /* Switch back to the slot corresponding to the root table */ + Assert(oldslot != NULL); + slot = oldslot; } } } diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index a9546106ce..da4c96a863 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -262,6 +262,7 @@ ExecInsert(ModifyTableState *mtstate, Relation resultRelationDesc; Oid newId; List *recheckIndexes = NIL; + TupleTableSlot *oldslot = NULL; /* * get the heap tuple out of the tuple table slot, making sure we have a @@ -318,7 +319,19 @@ ExecInsert(ModifyTableState *mtstate, map = mtstate->mt_partition_tupconv_maps[leaf_part_index]; if (map) { + Relation partrel = resultRelInfo->ri_RelationDesc; + tuple = do_convert_tuple(tuple, map); + + /* + * We must use the partition's tuple descriptor from this + * point on, until we're finished dealing with the partition. + * Use the dedicated slot for that. + */ + oldslot = slot; + slot = estate->es_partition_tuple_slot; + Assert(slot != NULL); + ExecSetSlotDescriptor(slot, RelationGetDescr(partrel)); ExecStoreTuple(tuple, slot, InvalidBuffer, true); } } @@ -566,6 +579,10 @@ ExecInsert(ModifyTableState *mtstate, { resultRelInfo = saved_resultRelInfo; estate->es_result_relation_info = resultRelInfo; + + /* Switch back to the slot corresponding to the root table */ + Assert(oldslot != NULL); + slot = oldslot; } /* @@ -1734,7 +1751,15 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) mtstate->mt_partitions = partitions; mtstate->mt_num_partitions = num_partitions; mtstate->mt_partition_tupconv_maps = partition_tupconv_maps; + + /* + * Initialize a dedicated slot to manipulate tuples of any given + * partition's rowtype. + */ + estate->es_partition_tuple_slot = ExecInitExtraTupleSlot(estate); } + else + estate->es_partition_tuple_slot = NULL; /* * Initialize any WITH CHECK OPTION constraints if needed. diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 703604ab9d..f49702b122 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -384,6 +384,9 @@ typedef struct EState TupleTableSlot *es_trig_oldtup_slot; /* for TriggerEnabled */ TupleTableSlot *es_trig_newtup_slot; /* for TriggerEnabled */ + /* Slot used to manipulate a tuple after it is routed to a partition */ + TupleTableSlot *es_partition_tuple_slot; + /* Parameter info: */ ParamListInfo es_param_list_info; /* values of external params */ ParamExecData *es_param_exec_vals; /* values of internal params */ -- 2.11.0