Re: [HACKERS] expanding inheritance in partition bound order

On 2017/08/17 10:09, Amit Langote wrote:
> On 2017/08/16 20:30, Amit Khandekar wrote:
>> On 16 August 2017 at 11:06, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> I am not
>> sure whether we are planning to commit these two things together or
>> incrementally :
>> 1. Expand in bound order
>> 2. Allow for locking only the partitioned tables first.
>>
>> For #1, the changes in pg_inherits.c are not required (viz, keeping
>> partitioned tables at the head of the list, adding inhchildparted
>> column, etc).
> 
> Yes.  Changes to pg_inherits.c can be committed completely independently
> of either 1 or 2, although then there would be nobody using that capability.
> 
> About 2: I think the capability to lock only the partitioned tables in
> expand_inherited_rtentry() will only be used once we have the patch to do
> the necessary planner restructuring that will allow us to defer child
> table locking to some place that is not expand_inherited_rtentry().  I am
> working on that patch now and should be able to show something soon.
> 
>> If we are going to do #2 together with #1, then in the patch set there
>> is no one using the capability introduced by #2. That is, there are no
>> callers of find_all_inheritors() that make use of the new
>> num_partitioned_children parameter. Also, there is no boolean
>> parameter  for find_all_inheritors() to be used to lock only the
>> partitioned tables.
>>
>> I feel we should think about
>> 0002-Teach-pg_inherits.c-a-bit-about-partitioning.patch later, and
>> first get the review done for the other patches.
> 
> I think that makes sense.

After thinking some more on this, I think Amit's suggestion to put this
patch at the end of the priority list is good (that is, the patch that
teaches pg_inherits infrastructure to list partitioned tables ahead in the
list.)  Its purpose is mainly to fulfill the requirement that partitioned
tables be able to be locked ahead of any leaf partitions in the list (per
the design idea Robert suggested [1]).  Patch which requires that
capability is not in the picture yet.  Perhaps, we could review and commit
this patch only when that future patch shows up.  So, I will hold that
patch for now.

Thoughts?

Attached rest of the patches.  0001 has changes per Ashutosh's review
comments [2]:

0001: Relieve RelationGetPartitionDispatchInfo() of doing any locking

0002: Teach expand_inherited_rtentry to use partition bound order

0003: Decouple RelationGetPartitionDispatchInfo() from executor

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/CA%2BTgmobwbh12OJerqAGyPEjb_%2B2y7T0nqRKTcjed6L4NTET6Fg%40mail.gmail.com

[2]
https://www.postgresql.org/message-id/CAFjFpRdXn7w7nVKv4qN9fa%2BtzRi_mJFNCsBWy%3Dbd0SLbYczUfA%40mail.gmail.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers