On 21 November 2017 at 17:24, Amit Khandekar <[email protected]> wrote: > On 13 November 2017 at 18:25, David Rowley <[email protected]> > wrote: >> >> 30. The following chunk of code is giving me a headache trying to >> verify which arrays are which size: >> >> ExecSetupPartitionTupleRouting(rel, >> mtstate->resultRelInfo, >> (operation == CMD_UPDATE ? nplans : 0), >> node->nominalRelation, >> estate, >> &partition_dispatch_info, >> &partitions, >> &partition_tupconv_maps, >> &subplan_leaf_map, >> &partition_tuple_slot, >> &num_parted, &num_partitions); >> mtstate->mt_partition_dispatch_info = partition_dispatch_info; >> mtstate->mt_num_dispatch = num_parted; >> mtstate->mt_partitions = partitions; >> mtstate->mt_num_partitions = num_partitions; >> mtstate->mt_parentchild_tupconv_maps = partition_tupconv_maps; >> mtstate->mt_subplan_partition_offsets = subplan_leaf_map; >> mtstate->mt_partition_tuple_slot = partition_tuple_slot; >> mtstate->mt_root_tuple_slot = MakeTupleTableSlot(); >> >> I know this patch is not completely responsible for it, but you're not >> making things any better. >> >> Would it not be better to invent some PartitionTupleRouting struct and >> make that struct a member of ModifyTableState and CopyState, then just >> pass the pointer to that struct to ExecSetupPartitionTupleRouting() >> and have it fill in the required details? I think the complexity of >> this is already on the high end, I think you really need to do the >> refactor before this gets any worse. >> > >Ok. I am currently working on doing this change. So not yet included in the >attached patch. Will send yet another revised patch for this change.
Attached incremental patch encapsulate_partinfo.patch (to be applied
over the latest v25 patch) contains changes to move all the
partition-related information into new structure
PartitionTupleRouting. Further to that, I also moved
PartitionDispatchInfo into this structure. So it looks like this :
typedef struct PartitionTupleRouting
{
PartitionDispatch *partition_dispatch_info;
int num_dispatch;
ResultRelInfo **partitions;
int num_partitions;
TupleConversionMap **parentchild_tupconv_maps;
int *subplan_partition_offsets;
TupleTableSlot *partition_tuple_slot;
TupleTableSlot *root_tuple_slot;
} PartitionTupleRouting;
So this structure now encapsulates *all* the
partition-tuple-routing-related information. So ModifyTableState now
has only one tuple-routing-related field 'mt_partition_tuple_routing'.
It is changed like this :
@@ -976,24 +976,14 @@ typedef struct ModifyTableState
TupleTableSlot *mt_existing; /* slot to store existing
target tuple in */
List *mt_excludedtlist; /* the excluded pseudo
relation's tlist */
TupleTableSlot *mt_conflproj; /* CONFLICT ... SET ...
projection target */
- struct PartitionDispatchData **mt_partition_dispatch_info;
- /* Tuple-routing support info */
- int mt_num_dispatch; /* Number of
entries in the above array */
- int mt_num_partitions; /* Number of
members in the following
-
* arrays */
- ResultRelInfo **mt_partitions; /* Per partition result
relation pointers */
- TupleTableSlot *mt_partition_tuple_slot;
- TupleTableSlot *mt_root_tuple_slot;
+ struct PartitionTupleRouting *mt_partition_tuple_routing; /*
Tuple-routing support info */
struct TransitionCaptureState *mt_transition_capture;
/* controls transition table population for specified operation */
struct TransitionCaptureState *mt_oc_transition_capture;
/* controls transition table population for INSERT...ON
CONFLICT UPDATE */
- TupleConversionMap **mt_parentchild_tupconv_maps;
- /* Per partition map for tuple conversion from root to leaf */
TupleConversionMap **mt_childparent_tupconv_maps;
/* Per plan/partition map for tuple conversion from child to root */
bool mt_is_tupconv_perpart; /* Is the above map
per-partition ? */
- int *mt_subplan_partition_offsets;
/* Stores position of update result rels in leaf partitions */
} ModifyTableState;
So the code in nodeModifyTable.c (and similar code in copy.c) is
accordingly adjusted to use mtstate->mt_partition_tuple_routing.
The places where we use (mtstate->mt_partition_dispatch_info != NULL)
condition to run tuple-routing code, I have replaced it with
mtstate->mt_partition_tuple_routing != NULL.
If you are ok with the incremental patch, I can extract this change
into a separate preparatory patch to be applied on PG master.
Thanks
-Amit Khandekar
encapsulate_partinfo.patch
Description: Binary data
