Re: [PATCH] Automatic HASH and LIST partition creation - Mailing list pgsql-hackers

There were many syntax proposals and finally, there is a patch realizing one of them. So I'd like to review it.

The syntax proposed in the patch seems good enough for me and is in accordance with one of the proposals in the discussion. Maybe I'd prefer using the word AUTOMATICALLY/AUTO instead of CONFIGURATION with explicit meaning that using this syntax we'd get already (automatically) created partitions and don't need to create them manually, as in the existing state of postgresql declarative partitioning. 

CREATE TABLE tbl (i int) PARTITION BY HASH (i) AUTOMATICALLY (MODULUS 3); (partitions are created automatically)

vs

CREATE TABLE tbl (i int) PARTITION BY HASH (i); (partitions should be created manually by use of PARTITION OF)

CREATE TABLE tbl (i char) PARTITION BY LIST (i) AUTOMATICALLY (VALUES IN ('a', 'b'), ('c', 'd'), ('e','f') DEFAULT PARTITION tbl_default);

vs

CREATE TABLE tbl (i char) PARTITION BY LIST (i); (partitions should be created manually by use of PARTITION OF)

I think this syntax can also be extended later with adding automatic creation of RANGE partitions, with IMMEDIATE/DEFERRED for dynamic/on-demand automatic partition creation, and with SUBPARTITION possibility.

But I don't have a strong preference for the word AUTOMATICALLY, moreover I saw opposition to using AUTO at the top of the discussion. I suppose we can go with the existing CONFIGURATION word.

If compare with existing declarative partitions, I think automatic creation simplifies the process for the end-user and  I'd vote for its committing into Postgres. The patch is short and clean in code style. It has enough comments Tests covering the new functionality are included. Yet it doesn't have documentation and I'd suppose it's worth adding it. Even if there will be syntax changes, I hope they will not be more than the replacement of several words. Current syntax is described in the text of a patch.

The patch applies cleanly and installcheck-world is passed. 

Some minor things:

I've got a compiler warning:

parse_utilcmd.c:4280:15: warning: unused variable 'lc' [-Wunused-variable]

When the number of partitions is over the maximum value of int32 the output shows a generic syntax error. I don't think it is very important as it is not the case someone will make deliberately, but maybe it's better to output something like "Partitions number is more than the maximum supported value"

ERROR:  syntax error at or near "888888888888" 

One more piece of nitpicking. Probably we can go just with a mention in documentation. 

create table test (i int, t text) partition by hash (i) configuration (modulus 8888);
ERROR:  out of shared memory
HINT:  You might need to increase max_locks_per_transaction.

Typo:

+ /* Add statemets to create each partition after we create parent table */

Overall I see the patch almost ready for commit and I'd like to meet this functionality in v14.

Tested it and see this feature very cool and much simpler to use compared to declarative partitioning to date.

Thanks!