On Wed, May 3, 2017 at 6:39 PM, amul sul <sula...@gmail.com> wrote:
> On Thu, Apr 27, 2017 at 1:42 AM, Robert Haas <robertmh...@gmail.com> wrote:
>>I spent some time today looking at these patches.  It seems like there
>>is some more work still needed here to produce something committable
>>regardless of which way we go, but I am inclined to think that Amul's
>>patch is a better basis for work going forward than Nagata-san's
>>patch. Here are some general comments on the two patches:
> Thanks for your time.
> [...]
>> - Neither patch contains any documentation updates, which is bad.
> Fixed in the attached version.

I have done an intial review of the patch and I have some comments.  I
will continue the review
and testing and report the results soon

Patch need to be rebased


if (key->strategy == PARTITION_STRATEGY_RANGE)
/* Disallow nulls in the range partition key of the tuple */
for (i = 0; i < key->partnatts; i++)
if (isnull[i])
errmsg("range partition key of row contains null")));

We need to add PARTITION_STRATEGY_HASH as well, we don't support NULL
for hash also, right?

RangeDatumContent **content;/* what's contained in each range bound datum?
  * (see the above enum); NULL for list
  * partitioned tables */

This will be NULL for hash as well we need to change the comments.

  bool has_null; /* Is there a null-accepting partition? false
  * for range partitioned tables */
  int null_index; /* Index of the null-accepting partition; -1

Comments needs to be changed for these two members as well

+/* One bound of a hash partition */
+typedef struct PartitionHashBound
+ int modulus;
+ int remainder;
+ int index;
+} PartitionHashBound;

It will good to add some comments to explain the structure members

Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

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

Reply via email to