Re: expanding inheritance in partition bound order

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: expanding inheritance in partition bound order
Date: 2017-09-11 09:56:22
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi Amit,

On 2017/09/11 16:16, Amit Khandekar wrote:
> Thanks Amit for the patch. I am still reviewing it, but meanwhile
> below are a few comments so far ...

Thanks for the review.

> + next_parted_idx += (list_length(*pds) - next_parted_idx - 1);
> I think this can be replaced just by :
> + next_parted_idx = list_length(*pds) - 1;
> Or, how about removing this variable next_parted_idx altogether ?
> Instead, we can just do this :
> pd->indexes[i] = -(1 + list_length(*pds));

That seems like the simplest possible way to do it.

> + next_leaf_idx += (list_length(*leaf_part_oids) - next_leaf_idx);
> Didn't understand why next_leaf_idx needs to be updated in case when
> the current partrelid is partitioned. I think it should be incremented
> only for leaf partitions, no ? Or for that matter, again, how about
> removing the variable 'next_leaf_idx' and doing this :
> *leaf_part_oids = lappend_oid(*leaf_part_oids, partrelid);
> pd->indexes[i] = list_length(*leaf_part_oids) - 1;


Attached updated patch does it that way for both partitioned table indexes
and leaf partition indexes. Thanks for pointing it out.

> -----------
> * For every partitioned table in the tree, starting with the root
> * partitioned table, add its relcache entry to parted_rels, while also
> * queuing its partitions (in the order in which they appear in the
> * partition descriptor) to be looked at later in the same loop. This is
> * a bit tricky but works because the foreach() macro doesn't fetch the
> * next list element until the bottom of the loop.
> I think the above comment needs to be modified with something
> explaining the relevant changed code. For e.g. there is no
> parted_rels, and the "tricky" part was there earlier because of the
> list being iterated and at the same time being appended.
> ------------

I think I forgot to update this comment.

> I couldn't see the existing comments like "Indexes corresponding to
> the internal partitions are multiplied by" anywhere in the patch. I
> think those comments are still valid, and important.

Again, I failed to keep this comment. Anyway, I reworded the comments a
bit to describe what the code is doing more clearly. Hope you find it so too.


Attachment Content-Type Size
0001-Decouple-RelationGetPartitionDispatchInfo-from-execu.patch text/plain 23.4 KB
0002-Make-RelationGetPartitionDispatch-expansion-order-de.patch text/plain 9.6 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Andrey Borodin 2017-09-11 10:25:15 Re: Allow GiST opcalsses without compress\decompres functions
Previous Message Christoph Berg 2017-09-11 09:53:38 Re: mysql_fdw + PG10: unrecognized node type: 217