Re: [HACKERS] [POC] hash partitioning

On Mon, Jul 3, 2017 at 4:39 PM, amul sul <sulamul@gmail.com> wrote:
> Thanks to catching this, fixed in the attached version.

Few comments on the latest version.

0001 looks fine, for 0002 I have some comments.

1.
+ hbounds = (PartitionHashBound * *) palloc(nparts *
+  sizeof(PartitionHashBound *));

/s/(PartitionHashBound * *)/(PartitionHashBound **)/g

2.
RelationBuildPartitionDesc
{    ....


* catalog scan that retrieved them, whereas that in the latter is
* defined by canonicalized representation of the list values or the
* range bounds.
*/
for (i = 0; i < nparts; i++)
result->oids[mapping[i]] = oids[i];

Should this comments mention about hash as well?

3.

if (b1->datums[b1->ndatums - 1][0] != b2->datums[b2->ndatums - 1][0])
return false;

if (b1->ndatums != b2->ndatums)
return false;

If ndatums itself is different then no need to access datum memory, so
better to check ndatum first.

4.
+ * next larger modulus.  For example, if you have a bunch
+ * of partitions that all have modulus 5, you can add a
+ * new new partition with modulus 10 or a new partition

Typo, "new new partition"  -> "new partition"


-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com