Re: [PATCH] Automatic HASH and LIST partition creation

Hi, 

Couple of comments: 

1. The syntax used omits the { IMMEDIATE | DEFERRED} keywords suggested in 

the earlier discussions. I think it is intuitive to include IMMEDIATE with the current implementation

so that the syntax can be extended with a  DEFERRED clause in future for dynamic partitions.

  CREATE TABLE tbl_lst (i int) PARTITION BY LIST (i)
 CONFIGURATION (values in (1, 2), (3, 4) DEFAULT PARTITION tbl_default);

 

After some consideration, I decided that we don't actually need to introduce IMMEDIATE | DEFERRED keyword. For hash and list partitions it will always be immediate, as the number of partitions cannot change after we initially set it. For range partitions, on the contrary, it doesn't make much sense to make partitions immediately, because in many use-cases one bound will be open.

As per discussions on this thread:   https://www.postgresql.org/message-id/flat/alpine.DEB.2.21.1907150711080.22273%40lancre

DEFERRED clause refers to creating partitions on the fly, while the data is being inserted. 

The number of partitions and partition bounds can be the same as specified initially 

during partitioned table creation, but the actual creation of partitions can be deferred. 

This seems like a potential extension to statically created partitions even in the case of 

hash and list partitions, as it won't involve moving any existing data.

Oh, now I see what you mean. The case with already existing tables will require changes to ALTER TABLE syntax. And that's where we may want to choose between immediate (i.e. locking) and deferred (i.e. concurrent) creation of partitions. I think we should try to implement it with existing keywords, maybe use 'CONCURRENTLY' keyword and it will look like:

ALTER TABLE tbl PARTITION BY ... CONFIGURATION (....) [CONCURRENTLY];

Anyway, the task of handling existing data is much more complicated, especially the 'concurrent' case and to be honest, I haven't put much thought into it yet.

The current patch only implements the simplest case of creating a new partitioned table. And I don't see if CREATE TABLE needs this immediate|deferred clause or if it will need it in the future.

Thoughts?

     2. One suggestion for generation of partition names is to append a unique id to

avoid conflicts.

Can you please give an example of such a conflict? I agree that current naming scheme is far from perfect, but I think that 'tablename'_partnum provides unique name for each partition.

Sorry for not being clear earlier, I mean the partition name 'tablename_partnum' can conflict with any existing table name. 

As per current impemetation, if I do the following it results in the table name conflict.

postgres=# create table tbl_test_5_1(i int);

CREATE TABLE

postgres=# CREATE TABLE tbl_test_5 (i int) PARTITION BY LIST((tbl_test_5))                                                                                                               CONFIGURATION (values in ('(1)'::tbl_test_5), ('(3)'::tbl_test_5) default partition tbl_default_5);

ERROR:  relation "tbl_test_5_1" already exists 

 

I don't mind adding some specific suffix for generated partitions, although it still may conflict with existing table names. The main disadvantage of this idea, is that it reduces number of symbols available for table name, which can lead to something like this:

CREATE TABLE parteddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd (a text, b int NOT NULL DEFAULT 0,  CONSTRAINT check_aa CHECK (length(a) > 0))
PARTITION BY LIST (a) CONFIGURATION (VALUES IN ('a','b'),('c','d') DEFAULT PARTITION parted_def) ;;
NOTICE:  identifier "parteddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd" will be truncated to "partedddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"
ERROR:  relation "partedddddddddddddddddddddddddddddddddddddddddddddddddddddddddd" already exists

The error message here is a bit confusing, as relation 'partedddddddddddddddddddddddddddddddddddddddddddddddddddddddddd' haven't existed before and this is a conflict between partitioned and generated partition table name. For now, I don't know if we can handle it more gracefully. Probably, we could truncate tablename to a shorter size, but it doesn't provide a complete solution, because partition number can contain several digits.

See also pg_partman documentation on the same issue: https://github.com/pgpartman/pg_partman/blob/master/doc/pg_partman.md#naming-length-limits

-- 
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company