> > 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>
v5-0001-Automatically-generate-partitions-by-LIST-and-HAS.patch
Description: Binary data