Re: partitioning code reorganization

On 2018/04/15 9:17, Alvaro Herrera wrote:
> Amit Langote wrote:
>> On Sat, Apr 14, 2018 at 11:48 PM, Amit Langote <amitlangote09@gmail.com> wrote:
>>> Hi.
>>>
>>> Thanks for taking care of few things I left like those PartitionKey
>>> accessors in rel.h.
>>
>> Forgot to mention -- there are some files that still include
>> catalog/partition.h but no longer need to.  Find a delta patch
>> attached that applies on your v6.
> 
> Thanks!  I pushed this now, putting back the enum in parsenodes and
> including this delta patch of yours.

Not sure if you were intending to discuss the remaining portion of the
changes I proposed last week (patch 0002 posted at [1]), but I'm posting
those patches here for discussion.  I've divided the patch further.

0001-Make-copying-of-cached-partitioning-info-more-con.patch

    Make copying of cached partitioning info more consistent

    Currently there are many callers that hold onto pointers that
    point into the partitioning related information cached in relcache.
    There are others, such as the planner, who copy important information
    before using it.

    Make everyone copy!

    Now because no part of the backend relies on the guarantee that
    pointers to partitioning info in relcache points to same memory even
    across relcache flushes, we don't need special guards as implemented
    in RelationClearRelation() to provide the aforementioned guarantee.

0002-Cache-all-partitioning-info-under-one-memory-cont.patch

    Cache all partitioning info under one memory context

    Instead of having one for PartitionKey and another for PartitionDesc,
    use just one.  Also, instead of allocating partition constraint
    expression tree directly under CacheMemoryContext, do it under the
    aforementioned context.

0003-Cache-partsupfunc-separately-from-PartitionKey.patch

    Cache partsupfunc separately from PartitionKey

    Callers who want to use partsupfunc now have to copy them separately
    from PartitionKey, which makes copying the latter a bit cheaper.


I think going the way of 0001 might seem counter to what we may *really*
want to do in this regard, which is to make it so that we can use (keep
around) the pointer to partition info in relcache, instead of copying the
information in it piece by piece for every query.  Robert's email from a
couple of months ago (that he also recently mentioned) brought this up wrt
to relcache data usage within the planner:

* RelOptInfo -> Relation *
https://www.postgresql.org/message-id/CA%2BTgmoYKToP4-adCFFRNrO21OGuH%3Dphx-fiB1dYoqksNYX6YHQ%40mail.gmail.com

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/CA%2BHiwqFo_NbJfS%2BY%3DtE94Tn5EVHXN02JkmGjwV4xT6fU3oc5OQ%40mail.gmail.com