Re: partition routing layering in nodeModifyTable.c

On 19/10/2020 07:54, Amit Langote wrote:
> On Sun, Oct 18, 2020 at 12:54 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> On 2020-Oct-17, Amit Langote wrote:
>>> Hmm, I don't see ri_PartitionCheckExpr as being a piece of routing
>>> information, because it's primarily meant to be used when inserting
>>> *directly* into a partition, although it's true we do initialize it in
>>> routing target partitions too in some cases.
>>>
>>> Also, ChildToRootMap was introduced by the trigger transition table
>>> project, not tuple routing.  I think we misjudged this when we added
>>> PartitionToRootMap to PartitionRoutingInfo, because it doesn't really
>>> belong there.  This patch fixes that by removing PartitionToRootMap.
>>>
>>> RootToPartitionMap and the associated partition slot is the only piece
>>> of extra information that is needed by tuple routing target relations.
>>
>> Well, I was thinking on making the ri_PartitionInfo be about
>> partitioning in general, not just specifically for partition tuple
>> routing.  Maybe Heikki is right that it may end up being simpler to
>> remove ri_PartitionInfo altogether.  It'd just be a couple of additional
>> pointers in ResultRelInfo after all.
> 
> So that's 2 votes for removing PartitionRoutingInfo from the tree.
> Okay, I have tried that in the attached 0002 patch.  Also, I fixed
> some comments in 0001 that still referenced PartitionToRootMap.

Pushed, with minor comment changes.

I also noticed that the way the getTargetResultRelInfo() helper function 
was used, was a bit messy. It was used when firing AFTER STATEMENT 
triggers, but for some reason the code to fire BEFORE STATEMENT triggers 
didn't use it but duplicated the logic instead. I made that a bit 
simpler, by always setting the rootResultRelInfo field in 
ExecInitModifyTable(), making the getTargetResultRelInfo() function 
unnecessary.

Thanks!

- Heikki