On 21 November 2017 at 17:24, Amit Khandekar <amitdkhan...@gmail.com> wrote: > On 13 November 2017 at 18:25, David Rowley <david.row...@2ndquadrant.com> > 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