Re: expanding inheritance in partition bound order

On 2017/08/18 4:54, Robert Haas wrote:
> On Thu, Aug 17, 2017 at 8:39 AM, Ashutosh Bapat
> <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>> [2] had a patch with some changes to the original patch you posted. I
>> didn't describe those changes in my mail, since they rearranged the
>> comments. Those changes are not part of this patch and you haven't
>> comments about those changes as well. If you have intentionally
>> excluded those changes, it's fine. In case, you haven't reviewed them,
>> please see if they are good to be incorporated.
>
> I took a quick look at your version but I think I like Amit's fine the
> way it is, so committed that and back-patched it to v10.

Thanks for committing.

> I find 0002 pretty ugly as things stand. We get a bunch of tuple maps
> that we don't really need, only to turn around and free them. We get
> a bunch of tuple slots that we don't need, only to turn around and
> drop them. We don't really need the PartitionDispatch objects either,
> except for the OIDs they contain. There's a lot of extra stuff being
> computed here that is really irrelevant for this purpose. I think we
> should try to clean that up somehow.

One way to do that might be to reverse the order of the remaining patches
and put the patch to refactor RelationGetPartitionDispatchInfo() first.
With that refactoring, PartitionDispatch itself has become much simpler in
that it does not contain a relcache reference to be closed eventually by
the caller, the tuple map, and the tuple table slot. Since those things
are required for tuple-routing, the refactoring makes
ExecSetupPartitionTupleRouting itself create them from the (minimal)
information returned by RelationGetPartitionDispatchInfo and ultimately
destroy when done using it. I kept the indexes field in
PartitionDispatchData though, because it's essentially free to create
while we are walking the partition tree in
RelationGetPartitionDispatchInfo() and it seems undesirable to make the
caller compute that information (indexes) by traversing the partition tree
all over again, if it doesn't otherwise have to. I am still considering
some counter-arguments raised by Amit Khandekar about this last assertion.

Thoughts?

Thanks,
Amit