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 patch. 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 part_key. - 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 partkeyexprs. - 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 build_joinrel_partition_info. 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: http://www.postgresql.org/mailpref/pgsql-hackers
