Re: Boolean partitions syntax - Mailing list pgsql-hackers

Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
> I think if this bug/open item can be resolved by adopting the minimal
> patch, then we should use it for that.  Maybe, we can discuss the rest of
> the changes independently.  If they make things better overall, we should
> definitely try to adopt them.

Yeah.  While I think that getting rid of the grammar restrictions on what
a partbound can be is a good idea, it seems like this is not the sort of
improvement to be making post-feature-freeze.  And it's certainly not
something to back-patch to v10.

I propose the attached slightly-less-invasive version of Amit's original
patch as what we should do in v10 and v11, and push the patch currently
under discussion out to v12.

> About the changes in transformPartitionBoundValue() to check for collation
> conflict, I think that seems unnecessary.

I agree.  We can document that the partbound expression is reduced to a
simple constant and leave it at that.  Nobody has yet been confused by
the possibility of putting COLLATE in a default expression, and I don't
believe that anybody will be confused here.

(Speaking of documentation, nobody seems to have noticed that
partition_bound_spec appears in alter_table.sgml too.)

            regards, tom lane

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 8c7b961..9ce59e3 100644
*** a/doc/src/sgml/ref/alter_table.sgml
--- b/doc/src/sgml/ref/alter_table.sgml
*************** ALTER TABLE [ IF EXISTS ] <replaceable c
*** 87,95 ****

  <phrase>and <replaceable class="parameter">partition_bound_spec</replaceable> is:</phrase>

! IN ( { <replaceable class="parameter">numeric_literal</replaceable> | <replaceable
class="parameter">string_literal</replaceable>| NULL } [, ...] ) | 
! FROM ( { <replaceable class="parameter">numeric_literal</replaceable> | <replaceable
class="parameter">string_literal</replaceable>| MINVALUE | MAXVALUE } [, ...] ) 
!   TO ( { <replaceable class="parameter">numeric_literal</replaceable> | <replaceable
class="parameter">string_literal</replaceable>| MINVALUE | MAXVALUE } [, ...] ) | 
  WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REMAINDER <replaceable
class="parameter">numeric_literal</replaceable>) 

  <phrase>and <replaceable class="parameter">column_constraint</replaceable> is:</phrase>
--- 87,95 ----

  <phrase>and <replaceable class="parameter">partition_bound_spec</replaceable> is:</phrase>

! IN ( { <replaceable class="parameter">numeric_literal</replaceable> | <replaceable
class="parameter">string_literal</replaceable>| TRUE | FALSE | NULL } [, ...] ) | 
! FROM ( { <replaceable class="parameter">numeric_literal</replaceable> | <replaceable
class="parameter">string_literal</replaceable>| TRUE | FALSE | MINVALUE | MAXVALUE } [, ...] ) 
!   TO ( { <replaceable class="parameter">numeric_literal</replaceable> | <replaceable
class="parameter">string_literal</replaceable>| TRUE | FALSE | MINVALUE | MAXVALUE } [, ...] ) | 
  WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REMAINDER <replaceable
class="parameter">numeric_literal</replaceable>) 

  <phrase>and <replaceable class="parameter">column_constraint</replaceable> is:</phrase>
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index f2bd562..763b4f5 100644
*** a/doc/src/sgml/ref/create_table.sgml
--- b/doc/src/sgml/ref/create_table.sgml
*************** CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY
*** 86,94 ****

  <phrase>and <replaceable class="parameter">partition_bound_spec</replaceable> is:</phrase>

! IN ( { <replaceable class="parameter">numeric_literal</replaceable> | <replaceable
class="parameter">string_literal</replaceable>| NULL } [, ...] ) | 
! FROM ( { <replaceable class="parameter">numeric_literal</replaceable> | <replaceable
class="parameter">string_literal</replaceable>| MINVALUE | MAXVALUE } [, ...] ) 
!   TO ( { <replaceable class="parameter">numeric_literal</replaceable> | <replaceable
class="parameter">string_literal</replaceable>| MINVALUE | MAXVALUE } [, ...] ) | 
  WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REMAINDER <replaceable
class="parameter">numeric_literal</replaceable>) 

  <phrase><replaceable class="parameter">index_parameters</replaceable> in <literal>UNIQUE</literal>, <literal>PRIMARY
KEY</literal>,and <literal>EXCLUDE</literal> constraints are:</phrase> 
--- 86,94 ----

  <phrase>and <replaceable class="parameter">partition_bound_spec</replaceable> is:</phrase>

! IN ( { <replaceable class="parameter">numeric_literal</replaceable> | <replaceable
class="parameter">string_literal</replaceable>| TRUE | FALSE | NULL } [, ...] ) | 
! FROM ( { <replaceable class="parameter">numeric_literal</replaceable> | <replaceable
class="parameter">string_literal</replaceable>| TRUE | FALSE | MINVALUE | MAXVALUE } [, ...] ) 
!   TO ( { <replaceable class="parameter">numeric_literal</replaceable> | <replaceable
class="parameter">string_literal</replaceable>| TRUE | FALSE | MINVALUE | MAXVALUE } [, ...] ) | 
  WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REMAINDER <replaceable
class="parameter">numeric_literal</replaceable>) 

  <phrase><replaceable class="parameter">index_parameters</replaceable> in <literal>UNIQUE</literal>, <literal>PRIMARY
KEY</literal>,and <literal>EXCLUDE</literal> constraints are:</phrase> 
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index e548476..6bfd22c 100644
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*************** partbound_datum:
*** 2799,2804 ****
--- 2799,2806 ----
              Sconst            { $$ = makeStringConst($1, @1); }
              | NumericOnly    { $$ = makeAConst($1, @1); }
              | NULL_P        { $$ = makeNullAConst(@1); }
+             | TRUE_P        { $$ = makeStringConst(pstrdup("t"), @1); }
+             | FALSE_P        { $$ = makeStringConst(pstrdup("f"), @1); }
          ;

  partbound_datum_list:
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 654464c..470fca0 100644
*** a/src/test/regress/expected/create_table.out
--- b/src/test/regress/expected/create_table.out
*************** Partition of: arrlp FOR VALUES IN ('{1}'
*** 885,887 ****
--- 885,901 ----
  Partition constraint: ((a IS NOT NULL) AND (((a)::anyarray OPERATOR(pg_catalog.=) '{1}'::integer[]) OR ((a)::anyarray
OPERATOR(pg_catalog.=)'{2}'::integer[]))) 

  DROP TABLE arrlp;
+ -- partition on boolean column
+ create table boolspart (a bool) partition by list (a);
+ create table boolspart_t partition of boolspart for values in (true);
+ create table boolspart_f partition of boolspart for values in (false);
+ \d+ boolspart
+                                  Table "public.boolspart"
+  Column |  Type   | Collation | Nullable | Default | Storage | Stats target | Description
+ --------+---------+-----------+----------+---------+---------+--------------+-------------
+  a      | boolean |           |          |         | plain   |              |
+ Partition key: LIST (a)
+ Partitions: boolspart_f FOR VALUES IN (false),
+             boolspart_t FOR VALUES IN (true)
+
+ drop table boolspart;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 5469434..140bf41 100644
*** a/src/test/regress/sql/create_table.sql
--- b/src/test/regress/sql/create_table.sql
*************** CREATE TABLE arrlp (a int[]) PARTITION B
*** 719,721 ****
--- 719,728 ----
  CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN ('{1}', '{2}');
  \d+ arrlp12
  DROP TABLE arrlp;
+
+ -- partition on boolean column
+ create table boolspart (a bool) partition by list (a);
+ create table boolspart_t partition of boolspart for values in (true);
+ create table boolspart_f partition of boolspart for values in (false);
+ \d+ boolspart
+ drop table boolspart;