On Mon, Jul 31, 2017 at 9:07 AM, Ashutosh Bapat
<ashutosh.ba...@enterprisedb.com> wrote:
> Forgot the patch set. Here it is.

The commit message for 0005 isn't really accurate given that it
follows 0004.  I think you could just flatten 0005 and 0006 into one

Reviewing those together:

- Existing code does partdesc = RelationGetPartitionDesc(relation) but
this has got it as part_desc.  Seems better to be consistent.
Likewise existing variables for PartitionKey are key or partkey, not

- get_relation_partition_info has a useless trailing return.

- Instead of adding nparts, boundinfo, and part_oids to RelOptInfo,
how about just adding partdesc?  Seems cleaner.

- pkexprs seems like a potentially confusing name, since PK is widely
used to mean "primary key" but here you mean "partition key".  Maybe

- build_simple_rel's matching algorithm is O(n^2).  We may have talked
about this problem before...

- This patch introduces some bits that are not yet used, like
nullable_pkexprs, or even the code to set the partition scheme for
joinrels.  I think perhaps some of that logic should be moved from
0008 to here - e.g. the initial portion of

There may be more, but I've run out of energy for tonight.

Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

Reply via email to