Re: [PATCH] Automatic HASH and LIST partition creation

Pavel Borisov <pashkin.elfe@gmail.com>

From: Pavel Borisov <pashkin.elfe@gmail.com>
To: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Thomas Munro <thomas.munro@gmail.com>, Anastasia Lubennikova <a.lubennikova@postgrespro.ru>, Rahila Syed <rahilasyed90@gmail.com>, Michael Paquier <michael@paquier.xyz>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Fabien COELHO <coelho@cri.ensmp.fr>, Robert Haas <robertmhaas@gmail.com>, Amul Sul <sulamul@gmail.com>
Date: 2021-07-09T10:29:49Z
Lists: pgsql-hackers

Attachments

>
> 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>

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Remove complaints about COLLATE clauses in partition bound values.