>
> I have reviewed the v4 patch. The patch does not get applied on the latest
> source. Kindly rebase.
> However I have found few comments.
>
> 1.
> > +-- must fail because of wrong configuration
> > +CREATE TABLE tbl_hash_fail (i int) PARTITION BY HASH (i)
> > +CONFIGURATION (values in (1, 2), (3, 4) default partition tbl_default);
>
> Here some of the keywords are mentioned in UPPER CASE (Ex: CREATE TABLE,
> CONFIGURATION, etc) and some are mentioned in lower case (Ex: values in,
> default partition, etc). Kindly make it common. I feel making it to UPPER
> CASE is better. Please take care of this in all the cases.
>
> 2. It is better to separate the failure cases and success cases in
> /src/test/regress/sql/create_table.sql for better readability. All the
> failure cases can be listed first and then the success cases.
>
> 3.
> > +           char *part_relname;
> > +
> > +           /*
> > +            * Generate partition name in the format:
> > +            * $relname_$partnum
> > +            * All checks of name validity will be made afterwards in
> DefineRelation()
> > +            */
> > +           part_relname = psprintf("%s_%d", cxt->relation->relname, i);
>
> The assignment can be done directly while declaring the variable.
>
Thank you for your review!
I've rebased the patch and made the changes mentioned.
PFA v5.

-- 
Best regards,
Pavel Borisov

Postgres Professional: http://postgrespro.com <http://www.postgrespro.com>

Attachment: v5-0001-Automatically-generate-partitions-by-LIST-and-HAS.patch
Description: Binary data

Reply via email to