Re: expanding inheritance in partition bound order

On 2017/08/30 12:03, Robert Haas wrote:
> On Tue, Aug 29, 2017 at 10:36 PM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>> I keep having the feeling that this is a big patch with a small patch
>>> struggling to get out.  Is it really necessary to change
>>> RelationGetPartitionDispatchInfo so much or could you just do a really
>>> minimal surgery to remove the code that sets the stuff we don't need?
>>> Like this:
>>
>> Sure, done in the attached updated patch.
> 
> On first glance, that looks pretty good.  I'll have a deeper look
> tomorrow.

Thanks.

> It strikes me that if PartitionTupleRoutingInfo is an
> executor structure, we should probably (as a separate patch) relocate
> FormPartitionKeyDatum and get_partition_for_tuple to someplace in
> src/backend/executor and rename the accordingly; maybe it's time for
> an execPartition.c? catalog/partition.c is getting really bug, so

I agree.

It would be a good idea to introduce an execPartition.c so that future
patches in this area (such as executor partition-pruning patch on the
horizon) have a convenient place to park their code.

Will see if I can make a patch for that.

> it's not a bad thing if some of that stuff gets moved elsewhere.  A
> lingering question in my mind, though, is whether it's really correct
> to think of PartitionTupleRoutingInfo as executor-specific.  Maybe
> we're going to need that for other things too?

Hmm.  Maybe, a subset of PartitionTupleRoutingInfo's fields are usable
outside the executor (only PartitionDispatch, which is exported by
partition.h anyway?), but not all of it.  For example, fields keystate,
tupslot seem pretty executor-specific.  In fact, they seem to be required
only for tuple routing.

One idea is to not introduce PartitionTupleRoutingInfo at all and add its
fields directly as ModifyTableState/CopyState fields.  We currently have,
for example, a mt_partition_tupconv_maps array with one slot for every
leaf partition.  Maybe, there could be following fields in
ModifyTableState (and similarly in CopyState):

int                   mt_num_parted    /* this one exists today */
struct PartitionDispatchData **mt_partition_dispatch_info; /* and this */
List                **mt_parted_keystate;
TupleConversionMap  **mt_parted_tupconv_maps;
TupleTableSlot      **mt_parted_tupslots;

Each of those arrays will have mt_num_parted slots.

Thanks,
Amit