Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kO12D-0005fn-3e for pgsql-hackers@arkaria.postgresql.org; Thu, 01 Oct 2020 16:02:21 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1kO12B-0007P6-71 for pgsql-hackers@arkaria.postgresql.org; Thu, 01 Oct 2020 16:02:19 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kO12A-0007Oy-Os for pgsql-hackers@lists.postgresql.org; Thu, 01 Oct 2020 16:02:18 +0000 Received: from mail.postgrespro.ru ([93.174.131.139]) by makus.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1kO11z-00072k-G9 for pgsql-hackers@lists.postgresql.org; Thu, 01 Oct 2020 16:02:17 +0000 Received: from [192.168.28.66] (cyclops.postgrespro.ru [93.174.131.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mail.postgrespro.ru (Postfix) with ESMTPSA id 4734321C3395; Thu, 1 Oct 2020 19:02:05 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=postgrespro.ru; s=mail; t=1601568125; bh=STzaZh/MBnJYObzSvWB0s6tkntTXS3q0MQG1AvmYm94=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=P4xZjAdF5bYczFFX/A5WuYYXgdofD1s5/Ea4vMnopVEDbHPNJqdB1Dvz2IeLHOx3X lKOctcb+rVdNqrXVKpLHPt2KvMo+JAhC7jiqSzUqeqUv1wvUG/so18AroB8YT8Gjht uFF0sLIzreA6unFQ9xnwG2ye2E/v5h1UR7zSQ+2Y= Subject: Re: [PATCH] Automatic HASH and LIST partition creation To: Rahila Syed Cc: Michael Paquier , Pavel Borisov , PostgreSQL Hackers , Fabien COELHO , Justin Pryzby , Robert Haas , Amul Sul References: <7fec3abb-c663-c0d2-8452-a46141be6d4a@postgrespro.ru> <7c4013db-a628-c8bd-ce54-064389109e0f@postgrespro.ru> <726c63e2-ef34-8113-f3cf-90e1646e4c2b@postgrespro.ru> <74a716a3-986d-c737-6217-6a3ffd488da3@postgrespro.ru> <20200924032738.GA28585@paquier.xyz> <37f1072a-16ec-8ce8-8044-1fcec85c8ed5@postgrespro.ru> From: Anastasia Lubennikova Message-ID: <199124d5-e710-71a6-1836-4653cd668d96@postgrespro.ru> Date: Thu, 1 Oct 2020 19:02:04 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/alternative; boundary="------------E4F44CDA23429EBC27AB4887" Content-Language: en-US List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk This is a multi-part message in MIME format. --------------E4F44CDA23429EBC27AB4887 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit On 30.09.2020 22:58, Rahila Syed wrote: > Hi Anastasia, > > I tested the syntax with some basic commands and it works fine, > regression tests also pass. > Thank you for your review. > 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. > 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. > > 3. Probably, here you mean to write list and hash instead of range and > list as > per the current state. > >       >      Range and list partitioning also support automatic creation > of partitions >       with an optional CONFIGURATION clause. >     > > 4. Typo in default_part_name > > +VALUES IN ( class="parameter">partition_bound_expr [, ...] ), [( > partition_bound_expr > [, ...] )] [, ...] [DEFAULT PARTITION class="parameter">defailt_part_name] > +MODULUS numeric_literal > > Yes, you're right. I will fix these typos in next version of the patch. > > Thank you, > Rahila Syed -- Anastasia Lubennikova Postgres Professional: http://www.postgrespro.com The Russian Postgres Company --------------E4F44CDA23429EBC27AB4887 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit
On 30.09.2020 22:58, Rahila Syed wrote:
Hi Anastasia,

I tested the syntax with some basic commands and it works fine, regression tests also pass.

Thank you for your review.
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.

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.


3. Probably, here you mean to write list and hash instead of range and list as 
per the current state. 
     <para>
     Range and list partitioning also support automatic creation of partitions
      with an optional <literal>CONFIGURATION</literal> clause.
    </para>
 
4. Typo in default_part_name

+VALUES IN ( <replaceable class="parameter">partition_bound_expr</replaceable> [, ...] ), [( <replaceable class="parameter">partition_bound_expr</replaceable> [, ...] )] [, ...] [DEFAULT PARTITION <replaceable class="parameter">defailt_part_name</replaceable>]
+MODULUS <replaceable class="parameter">numeric_literal</replaceable>

Yes, you're right. I will fix these typos in next version of the patch.

Thank you,
Rahila Syed


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