Re: [Sender Address Forgery]Re: using expression syntax for partition bounds

[Amit Langote]

Hi Peter,

On 2019/01/26 17:25, Peter Eisentraut wrote:
> On 25/01/2019 16:19, Tom Lane wrote:
>> Peter Eisentraut  writes:
>>> committed
>>
>> Some of the buildfarm members are having sort-ordering problems
>> with this.  Looks like you could work around it with different
>> partition names (don't assume the relative sort order of
>> letters and digits).
> 
> Fixed by light renaming.

Thank you for committing and taking care of BF failures.

Regards,
Amit

Re: using expression syntax for partition bounds

[Peter Eisentraut]

On 25/01/2019 16:19, Tom Lane wrote:
> Peter Eisentraut  writes:
>> committed
> 
> Some of the buildfarm members are having sort-ordering problems
> with this.  Looks like you could work around it with different
> partition names (don't assume the relative sort order of
> letters and digits).

Fixed by light renaming.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: using expression syntax for partition bounds

[Michael Paquier]

On Sat, Jan 26, 2019 at 12:14:33PM +0900, Amit Langote wrote:
> The describe lines are there just to show that the stored expessions are
> not verbatim same as the input expressions, so it seemed an overkill to add
> them for all of the partitions.

I see, so per 7c079d7 this is the reason why showing part_3 matters
because you want to show the result of the expression after executing
the DDL, and this has been just added:
+CREATE TABLE part_3 PARTITION OF list_parted FOR VALUES IN ((2+1));

I think that it would be a good thing to show at least the NULL
partition because its partition definition has semantics different
from the three others so as it replaces part_1.  What do you think?
--
Michael


signature.asc
Description: PGP signature

Re: using expression syntax for partition bounds

[Amit Langote]

 Hi,

On Sat, Jan 26, 2019 at 12:01 Michael Paquier  wrote:

> On Sat, Jan 26, 2019 at 03:14:51AM +0900, Amit Langote wrote:
> > How about replacing \d+ list_parted with couple of \d on individual
> > partitions, like in the attached?
>
> That would make it.  Why just part_1 and part_3 though?  It looks more
> complete to add part_null and part_2 as well.


The describe lines are there just to show that the stored expessions are
not verbatim same as the input expressions, so it seemed an overkill to add
them for all of the partitions.

Thanks,
Amit

PS: away from a computer, typing on the smartphone

>

Re: using expression syntax for partition bounds

[Michael Paquier]

On Sat, Jan 26, 2019 at 03:14:51AM +0900, Amit Langote wrote:
> How about replacing \d+ list_parted with couple of \d on individual
> partitions, like in the attached?

That would make it.  Why just part_1 and part_3 though?  It looks more
complete to add part_null and part_2 as well.
--
Michael


signature.asc
Description: PGP signature

Re: using expression syntax for partition bounds

[Amit Langote]

On Sat, Jan 26, 2019 at 12:19 AM Tom Lane  wrote:
> Peter Eisentraut  writes:
> > committed
>
> Some of the buildfarm members are having sort-ordering problems
> with this.  Looks like you could work around it with different
> partition names (don't assume the relative sort order of
> letters and digits).

How about replacing \d+ list_parted with couple of \d on individual
partitions, like in the attached?

Thanks,
Amit


7c079d7417-stabilize-test-output.patch
Description: Binary data

Re: using expression syntax for partition bounds

[Tom Lane]

Peter Eisentraut  writes:
> committed

Some of the buildfarm members are having sort-ordering problems
with this.  Looks like you could work around it with different
partition names (don't assume the relative sort order of
letters and digits).

regards, tom lane

Re: using expression syntax for partition bounds

[Peter Eisentraut]

On 24/01/2019 13:57, Amit Langote wrote:
> The if (contain_var_clause(value)) block is new code, but I agree its
> ereport should have parser_errposition just like other ereports in that
> function.  Fixed that in the attached.

committed

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: using expression syntax for partition bounds

[Amit Langote]

Hi,

Thanks for looking.

On 2019/01/24 21:00, Alvaro Herrera wrote:
> Why did you lose the parser_errposition in parse_utilcmd.c line 3854?
> 
>> -/* Fail if we don't have a constant (i.e., non-immutable coercion) */
>> -if (!IsA(value, Const))
>> +/* Make sure the expression does not refer to any vars. */
>> +if (contain_var_clause(value))
>>  ereport(ERROR,
>> -(errcode(ERRCODE_DATATYPE_MISMATCH),
>> - errmsg("specified value cannot be cast to type 
>> %s for column \"%s\"",
>> -format_type_be(colType), 
>> colName),
>> - errdetail("The cast requires a non-immutable 
>> conversion."),
>> - errhint("Try putting the literal value in 
>> single quotes."),
>> - parser_errposition(pstate, con->location)));
>> +(errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
>> + errmsg("cannot use column references in 
>> partition bound expression")));

The if (contain_var_clause(value)) block is new code, but I agree its
ereport should have parser_errposition just like other ereports in that
function.  Fixed that in the attached.

Thanks,
Amit
From ae4fbcaa97364e1a33c5c25eb983a23d3acad30c Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 6 Jul 2018 14:05:22 +0900
Subject: [PATCH v11] Allow generalized expression syntax for partition bounds

Authors: Kyotaro Horiguchi, Tom Lane, Amit Langote
---
 doc/src/sgml/ref/alter_table.sgml  |   6 +-
 doc/src/sgml/ref/create_table.sgml |  19 +--
 src/backend/commands/tablecmds.c   |   9 ++
 src/backend/optimizer/util/clauses.c   |   4 +-
 src/backend/parser/gram.y  |  60 +---
 src/backend/parser/parse_agg.c |  10 ++
 src/backend/parser/parse_expr.c|   5 +
 src/backend/parser/parse_func.c|   3 +
 src/backend/parser/parse_utilcmd.c | 215 +++--
 src/include/optimizer/clauses.h|   2 +
 src/include/parser/parse_node.h|   3 +-
 src/include/utils/partcache.h  |   6 +
 src/test/regress/expected/create_table.out |  91 +---
 src/test/regress/sql/create_table.sql  |  51 ++-
 14 files changed, 315 insertions(+), 169 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml 
b/doc/src/sgml/ref/alter_table.sgml
index 0d1feaf828..0aa0f093f2 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -86,9 +86,9 @@ ALTER TABLE [ IF EXISTS ] name
 
 and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | TRUE | FALSE | 
NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 and column_constraint 
is:
diff --git a/doc/src/sgml/ref/create_table.sgml 
b/doc/src/sgml/ref/create_table.sgml
index 857515ec8f..22dbc07b23 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -87,9 +87,9 @@ class="parameter">referential_action ] [ ON 
UPDATE and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | TRUE | FALSE | 
NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 index_parameters in 
UNIQUE, PRIMARY KEY, and 
EXCLUDE constraints are:
@@ -413,12 +413,13 @@ WITH ( MODULUS numeric_literal, REM
  
 
  
-  Each of the values specified in
-  the partition_bound_spec is
-  a literal, NULL, MINVALUE, or
-  MAXVALUE.  Each literal value must be either a
-  numeric constant that is coercible to the corresponding partition key
-  column's type, or a string literal that is valid input for that type.
+  partition_bound_expr is
+  any variable-free expression (subqueries, window functions, aggregate
+  functions, and set-returning functions are not allowed).  Its data type
+  must match the data type of the corresponding partition key column.
+  The expression is evaluated once at table creation time, so it can
+  even contain volatile expressions such as
+  CURRENT_TIMESTAMP.
  
 
  
diff --git a/src/backend/commands/tablecmds.c 

Re: using expression syntax for partition bounds

[Alvaro Herrera]

Why did you lose the parser_errposition in parse_utilcmd.c line 3854?

> - /* Fail if we don't have a constant (i.e., non-immutable coercion) */
> - if (!IsA(value, Const))
> + /* Make sure the expression does not refer to any vars. */
> + if (contain_var_clause(value))
>   ereport(ERROR,
> - (errcode(ERRCODE_DATATYPE_MISMATCH),
> -  errmsg("specified value cannot be cast to type 
> %s for column \"%s\"",
> - format_type_be(colType), 
> colName),
> -  errdetail("The cast requires a non-immutable 
> conversion."),
> -  errhint("Try putting the literal value in 
> single quotes."),
> -  parser_errposition(pstate, con->location)));
> + (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
> +  errmsg("cannot use column references in 
> partition bound expression")));


-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: using expression syntax for partition bounds

[Amit Langote]

Horiguchi-san,

Thanks for checking.

On 2019/01/24 19:03, Kyotaro HORIGUCHI wrote:
> At Fri, 18 Jan 2019 17:50:56 +0900, Amit Langote wrote:
>> On 2019/01/18 16:48, Peter Eisentraut wrote:
 How about the following note in the documentation:

 +  Although volatile expressions such as
 +  CURRENT_TIMESTAMP can be 
 used
 +  for this, be careful when using them, because
 +  PostgreSQL will evaluate them only once
 +  when adding the partition.
>>>
>>> I don't think we have to phrase it in this warning way.  Just say that
>>> volatile expressions are evaluated at the time of the DDL statement.
>>
>> OK, then just this:
>>
>> +  Even volatile expressions such as
>> +  CURRENT_TIMESTAMP can be used.
> 
> I agree to not to phrase in a warning way, but it seems
> too-simplified. I think the reason is still required, like this?:
> 
> ===
> The expression is evaluated once at the table creation time so it
> can involve even volatile expressions such as
> CURRENT_TIMESTAMP.

Ah, that's perhaps a better way of describing this feature.

Attached rebased patch containing above change.

Thanks,
Amit
From 6ed90ad9640c4d7bfe53c280535631c45d2cafae Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 6 Jul 2018 14:05:22 +0900
Subject: [PATCH v10] Allow generalized expression syntax for partition bounds

Authors: Kyotaro Horiguchi, Tom Lane, Amit Langote
---
 doc/src/sgml/ref/alter_table.sgml  |   6 +-
 doc/src/sgml/ref/create_table.sgml |  19 +--
 src/backend/commands/tablecmds.c   |   9 ++
 src/backend/optimizer/util/clauses.c   |   4 +-
 src/backend/parser/gram.y  |  60 +---
 src/backend/parser/parse_agg.c |  10 ++
 src/backend/parser/parse_expr.c|   5 +
 src/backend/parser/parse_func.c|   3 +
 src/backend/parser/parse_utilcmd.c | 214 +++--
 src/include/optimizer/clauses.h|   2 +
 src/include/parser/parse_node.h|   3 +-
 src/include/utils/partcache.h  |   6 +
 src/test/regress/expected/create_table.out |  89 +---
 src/test/regress/sql/create_table.sql  |  51 ++-
 14 files changed, 312 insertions(+), 169 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml 
b/doc/src/sgml/ref/alter_table.sgml
index 0d1feaf828..0aa0f093f2 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -86,9 +86,9 @@ ALTER TABLE [ IF EXISTS ] name
 
 and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | TRUE | FALSE | 
NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 and column_constraint 
is:
diff --git a/doc/src/sgml/ref/create_table.sgml 
b/doc/src/sgml/ref/create_table.sgml
index 857515ec8f..22dbc07b23 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -87,9 +87,9 @@ class="parameter">referential_action ] [ ON 
UPDATE and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | TRUE | FALSE | 
NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 index_parameters in 
UNIQUE, PRIMARY KEY, and 
EXCLUDE constraints are:
@@ -413,12 +413,13 @@ WITH ( MODULUS numeric_literal, REM
  
 
  
-  Each of the values specified in
-  the partition_bound_spec is
-  a literal, NULL, MINVALUE, or
-  MAXVALUE.  Each literal value must be either a
-  numeric constant that is coercible to the corresponding partition key
-  column's type, or a string literal that is valid input for that type.
+  partition_bound_expr is
+  any variable-free expression (subqueries, window functions, aggregate
+  functions, and set-returning functions are not allowed).  Its data type
+  must match the data type of the corresponding partition key column.
+  The expression is evaluated once at table creation time, so it can
+  even contain volatile expressions such as
+  CURRENT_TIMESTAMP.
  
 
  
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 738c178107..e338e760ab 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ 

Re: using expression syntax for partition bounds

[Kyotaro HORIGUCHI]

Hello.

At Fri, 18 Jan 2019 17:50:56 +0900, Amit Langote 
 wrote in 

> Thanks for the comments.
> 
> On 2019/01/18 16:48, Peter Eisentraut wrote:
> >> How about the following note in the documentation:
> >>
> >> +  Although volatile expressions such as
> >> +  CURRENT_TIMESTAMP can be 
> >> used
> >> +  for this, be careful when using them, because
> >> +  PostgreSQL will evaluate them only once
> >> +  when adding the partition.
> > 
> > I don't think we have to phrase it in this warning way.  Just say that
> > volatile expressions are evaluated at the time of the DDL statement.
> 
> OK, then just this:
> 
> +  Even volatile expressions such as
> +  CURRENT_TIMESTAMP can be used.

I agree to not to phrase in a warning way, but it seems
too-simplified. I think the reason is still required, like this?:

===
The expression is evaluated once at the table creation time so it
can involve even volatile expressions such as
CURRENT_TIMESTAMP.
===

> >> Sorry but I'm not sure how or what I would test about this.  Maybe, just
> >> add a test in create_table.sql/alter_table.sql that shows that using
> >> volatile expression doesn't cause an error?
> > 
> > Possibilities: Create a range partition with current_timestamp as the
> > upper bound and then in a separate transaction insert current_timestamp
> > and have it appear in the default partition.  Or create list partition
> > with session_user as one partition's value and then insert session_user
> > and have it appear in that table.  Or something like those.
> 
> OK, added a test that uses current_timestamp.
> 
> >> So, should the "name" type's collation should simply be discarded in favor
> >> of "POSIX" that's being used for partitioning?
> > 
> > In that specific case, yes, I think so.
> > 
> >>> What we don't want is someone writing an explicit COLLATE clause.  I
> >>> think we just need to check that there is no top-level COLLATE clause.
> >>> This would then sort of match the logic parse_collate.c for combining
> >>> collations.
> >>
> >> Maybe I'm missing something, but isn't it OK to allow the COLLATE clause
> >> as long as it specifies the matching collation as the parent?
> > 
> > Yes, that should be OK.
> 
> Alright, I've included a test that uses cast expression in partition bound.
> 
> Updated patch attached.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Re: using expression syntax for partition bounds

[Amit Langote]

Thanks for the comments.

On 2019/01/18 16:48, Peter Eisentraut wrote:
>> How about the following note in the documentation:
>>
>> +  Although volatile expressions such as
>> +  CURRENT_TIMESTAMP can be used
>> +  for this, be careful when using them, because
>> +  PostgreSQL will evaluate them only once
>> +  when adding the partition.
> 
> I don't think we have to phrase it in this warning way.  Just say that
> volatile expressions are evaluated at the time of the DDL statement.

OK, then just this:

+  Even volatile expressions such as
+  CURRENT_TIMESTAMP can be used.

>> Sorry but I'm not sure how or what I would test about this.  Maybe, just
>> add a test in create_table.sql/alter_table.sql that shows that using
>> volatile expression doesn't cause an error?
> 
> Possibilities: Create a range partition with current_timestamp as the
> upper bound and then in a separate transaction insert current_timestamp
> and have it appear in the default partition.  Or create list partition
> with session_user as one partition's value and then insert session_user
> and have it appear in that table.  Or something like those.

OK, added a test that uses current_timestamp.

>> So, should the "name" type's collation should simply be discarded in favor
>> of "POSIX" that's being used for partitioning?
> 
> In that specific case, yes, I think so.
> 
>>> What we don't want is someone writing an explicit COLLATE clause.  I
>>> think we just need to check that there is no top-level COLLATE clause.
>>> This would then sort of match the logic parse_collate.c for combining
>>> collations.
>>
>> Maybe I'm missing something, but isn't it OK to allow the COLLATE clause
>> as long as it specifies the matching collation as the parent?
> 
> Yes, that should be OK.

Alright, I've included a test that uses cast expression in partition bound.

Updated patch attached.

Thanks,
Amit
>From f68a0c72c68dd096182be751b238621b3a8c6741 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 6 Jul 2018 14:05:22 +0900
Subject: [PATCH v9] Allow generalized expression syntax for partition bounds

Authors: Kyotaro Horiguchi, Tom Lane, Amit Langote
---
 doc/src/sgml/ref/alter_table.sgml  |   6 +-
 doc/src/sgml/ref/create_table.sgml |  18 +--
 src/backend/commands/tablecmds.c   |   9 ++
 src/backend/optimizer/util/clauses.c   |   4 +-
 src/backend/parser/gram.y  |  60 +---
 src/backend/parser/parse_agg.c |  10 ++
 src/backend/parser/parse_expr.c|   5 +
 src/backend/parser/parse_func.c|   3 +
 src/backend/parser/parse_utilcmd.c | 214 +++--
 src/include/optimizer/clauses.h|   2 +
 src/include/parser/parse_node.h|   1 +
 src/include/utils/partcache.h  |   6 +
 src/test/regress/expected/create_table.out |  89 +---
 src/test/regress/sql/create_table.sql  |  51 ++-
 14 files changed, 310 insertions(+), 168 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml 
b/doc/src/sgml/ref/alter_table.sgml
index 0d1feaf828..0aa0f093f2 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -86,9 +86,9 @@ ALTER TABLE [ IF EXISTS ] name
 
 and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | TRUE | FALSE | 
NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 and column_constraint 
is:
diff --git a/doc/src/sgml/ref/create_table.sgml 
b/doc/src/sgml/ref/create_table.sgml
index 857515ec8f..4230f6947a 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -87,9 +87,9 @@ class="parameter">referential_action ] [ ON 
UPDATE and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | TRUE | FALSE | 
NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 index_parameters in 
UNIQUE, PRIMARY KEY, and 
EXCLUDE constraints are:
@@ -413,12 +413,12 @@ WITH ( MODULUS numeric_literal, REM
  
 
  
-  Each of the values specified in
-  the partition_bound_spec is
-  a literal, NULL, MINVALUE, or
-  MAXVALUE.  Each literal value must be either a
-  numeric constant that is 

Re: using expression syntax for partition bounds

[Peter Eisentraut]

On 16/01/2019 08:41, Amit Langote wrote:
> OK, will change it back to partition_bound_expr.  Removing "bound" from it
> makes the term ambiguous?

Yeah, let's leave it in.

> How about the following note in the documentation:
> 
> +  Although volatile expressions such as
> +  CURRENT_TIMESTAMP can be used
> +  for this, be careful when using them, because
> +  PostgreSQL will evaluate them only once
> +  when adding the partition.

I don't think we have to phrase it in this warning way.  Just say that
volatile expressions are evaluated at the time of the DDL statement.

> Sorry but I'm not sure how or what I would test about this.  Maybe, just
> add a test in create_table.sql/alter_table.sql that shows that using
> volatile expression doesn't cause an error?

Possibilities: Create a range partition with current_timestamp as the
upper bound and then in a separate transaction insert current_timestamp
and have it appear in the default partition.  Or create list partition
with session_user as one partition's value and then insert session_user
and have it appear in that table.  Or something like those.

>> I think that needs more refinement.  In v8, the following errors
>>
>> CREATE TABLE t2 ( a name COLLATE "POSIX" ) PARTITION BY RANGE (a);
>> CREATE TABLE t2a PARTITION OF t2 FOR VALUES FROM (name 'foo') TO (name
>> 'xyz');
>> ERROR:  collation of partition bound value for column "a" does not match
>> partition key collation "POSIX"
>>
>> The problem here is that the "name" type has a collation that is neither
>> the one of the column nor the default collation.  We can allow that.
> 
> So, should the "name" type's collation should simply be discarded in favor
> of "POSIX" that's being used for partitioning?

In that specific case, yes, I think so.

>> What we don't want is someone writing an explicit COLLATE clause.  I
>> think we just need to check that there is no top-level COLLATE clause.
>> This would then sort of match the logic parse_collate.c for combining
>> collations.
> 
> Maybe I'm missing something, but isn't it OK to allow the COLLATE clause
> as long as it specifies the matching collation as the parent?

Yes, that should be OK.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: using expression syntax for partition bounds

[Amit Langote]

Thanks for the review.

On 2019/01/15 22:24, Peter Eisentraut wrote:
> On 15/01/2019 07:31, Amit Langote wrote:
>>> Is "partition bound" the right term?  For list partitioning, it's not
>>> really a bound.  Maybe it's OK.
>>
>> Hmm, maybe "partition bound value"?  Just want to stress that the
>> expression specifies "bounding" value of a partition.
> 
> I was more concerned about the term "bound", which it is not range
> partitioning.  But I can't think of a better term.
> 
> I wouldn't change expr to value as you have done between v7 and v8,
> since the point of this patch is to make expressions valid where
> previously only values were.

OK, will change it back to partition_bound_expr.  Removing "bound" from it
makes the term ambiguous?

>>> I don't see any treatment of non-immutable expressions.  There is a test
>>> case with a non-immutable cast that was removed, but that's all.  There
>>> was considerable discussion earlier in the thread on whether
>>> non-immutable expressions should be allowed.  I'm not sure what the
>>> resolution was, but in any case there should be documentation and tests
>>> of the outcome.
>>
>> The partition bound expression is evaluated only once, that is, when the
>> partition creation command is executed, and what gets stored in the
>> catalog is a Const expression that contains the value that was computed,
>> not the original non-immutable expression.  So, we don't need to do
>> anything special in terms of code to handle users possibly specifying a
>> non-immutable expression as partition bound.  Tom said something in that
>> thread which seems to support such a position:
>>
>> https://www.postgresql.org/message-id/22534.1523374457%40sss.pgh.pa.us
> 
> OK, if we are going with that approach, then it needs to be documented
> and there should be test cases.

How about the following note in the documentation:

+  Although volatile expressions such as
+  CURRENT_TIMESTAMP can be used
+  for this, be careful when using them, because
+  PostgreSQL will evaluate them only once
+  when adding the partition.

Sorry but I'm not sure how or what I would test about this.  Maybe, just
add a test in create_table.sql/alter_table.sql that shows that using
volatile expression doesn't cause an error?

>>> The collation handling might need another look.  The following is
>>> allowed without any diagnostic:
>>>
>>> CREATE TABLE t2 (
>>> a text COLLATE "POSIX"
>>> ) PARTITION BY RANGE (a);
>>> CREATE TABLE t2a PARTITION OF t2 FOR VALUES FROM ('foo' COLLATE "C") TO
>>> ('xyz');
>>>
>>> I think the correct behavior would be that an explicit collation in the
>>> partition bound expression is an error.
> 
>> But that's essentially ignoring the collation specified by the user for
>> the partition bound value without providing any information about that to
>> the user.  I've fixed that by explicitly checking if the collate clause in
>> the partition bound value expression contains the same collation as the
>> parent partition key collation and give an error otherwise.
> 
> I think that needs more refinement.  In v8, the following errors
> 
> CREATE TABLE t2 ( a name COLLATE "POSIX" ) PARTITION BY RANGE (a);
> CREATE TABLE t2a PARTITION OF t2 FOR VALUES FROM (name 'foo') TO (name
> 'xyz');
> ERROR:  collation of partition bound value for column "a" does not match
> partition key collation "POSIX"
> 
> The problem here is that the "name" type has a collation that is neither
> the one of the column nor the default collation.  We can allow that.

So, should the "name" type's collation should simply be discarded in favor
of "POSIX" that's being used for partitioning?

> What we don't want is someone writing an explicit COLLATE clause.  I
> think we just need to check that there is no top-level COLLATE clause.
> This would then sort of match the logic parse_collate.c for combining
> collations.

Maybe I'm missing something, but isn't it OK to allow the COLLATE clause
as long as it specifies the matching collation as the parent?  So:

CREATE TABLE t2b PARTITION OF t2 FOR VALUES FROM (name 'bar' collate "C")
TO (name 'foo');
ERROR:  collation of partition bound value for column "a" does not match
partition key collation "POSIX"

-- either of the following is ok

CREATE TABLE t2b PARTITION OF t2 FOR VALUES FROM (name 'bar' collate
"POSIX") TO (name 'foo');

CREATE TABLE t2b PARTITION OF t2 FOR VALUES FROM (name 'bar') TO (name 'foo');

Thanks,
Amit

Re: using expression syntax for partition bounds

[Peter Eisentraut]

On 15/01/2019 07:31, Amit Langote wrote:
>> Is "partition bound" the right term?  For list partitioning, it's not
>> really a bound.  Maybe it's OK.
> 
> Hmm, maybe "partition bound value"?  Just want to stress that the
> expression specifies "bounding" value of a partition.

I was more concerned about the term "bound", which it is not range
partitioning.  But I can't think of a better term.

I wouldn't change expr to value as you have done between v7 and v8,
since the point of this patch is to make expressions valid where
previously only values were.

>> I don't see any treatment of non-immutable expressions.  There is a test
>> case with a non-immutable cast that was removed, but that's all.  There
>> was considerable discussion earlier in the thread on whether
>> non-immutable expressions should be allowed.  I'm not sure what the
>> resolution was, but in any case there should be documentation and tests
>> of the outcome.
> 
> The partition bound expression is evaluated only once, that is, when the
> partition creation command is executed, and what gets stored in the
> catalog is a Const expression that contains the value that was computed,
> not the original non-immutable expression.  So, we don't need to do
> anything special in terms of code to handle users possibly specifying a
> non-immutable expression as partition bound.  Tom said something in that
> thread which seems to support such a position:
> 
> https://www.postgresql.org/message-id/22534.1523374457%40sss.pgh.pa.us

OK, if we are going with that approach, then it needs to be documented
and there should be test cases.

>> The collation handling might need another look.  The following is
>> allowed without any diagnostic:
>>
>> CREATE TABLE t2 (
>> a text COLLATE "POSIX"
>> ) PARTITION BY RANGE (a);
>> CREATE TABLE t2a PARTITION OF t2 FOR VALUES FROM ('foo' COLLATE "C") TO
>> ('xyz');
>>
>> I think the correct behavior would be that an explicit collation in the
>> partition bound expression is an error.

> But that's essentially ignoring the collation specified by the user for
> the partition bound value without providing any information about that to
> the user.  I've fixed that by explicitly checking if the collate clause in
> the partition bound value expression contains the same collation as the
> parent partition key collation and give an error otherwise.

I think that needs more refinement.  In v8, the following errors

CREATE TABLE t2 ( a name COLLATE "POSIX" ) PARTITION BY RANGE (a);
CREATE TABLE t2a PARTITION OF t2 FOR VALUES FROM (name 'foo') TO (name
'xyz');
ERROR:  collation of partition bound value for column "a" does not match
partition key collation "POSIX"

The problem here is that the "name" type has a collation that is neither
the one of the column nor the default collation.  We can allow that.
What we don't want is someone writing an explicit COLLATE clause.  I
think we just need to check that there is no top-level COLLATE clause.
This would then sort of match the logic parse_collate.c for combining
collations.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: using expression syntax for partition bounds

[Amit Langote]

Thanks for the review and sorry it took me a while to reply.

On 2019/01/02 22:58, Peter Eisentraut wrote:
> On 26/11/2018 05:58, Amit Langote wrote:
>> On 2018/11/09 14:38, Amit Langote wrote:
>>> Rebased due to change in addRangeTableEntryForRelation's API.
>>
>> Rebased again due to changes in psql's describe output for partitioned 
>> tables.
> 
> Review:
> 
> Is "partition bound" the right term?  For list partitioning, it's not
> really a bound.  Maybe it's OK.

Hmm, maybe "partition bound value"?  Just want to stress that the
expression specifies "bounding" value of a partition.

> Keep the ordering of EXPR_KIND_PARTITION_BOUND in the various switch
> statements consistent.

OK, fixed.

> I don't see any treatment of non-immutable expressions.  There is a test
> case with a non-immutable cast that was removed, but that's all.  There
> was considerable discussion earlier in the thread on whether
> non-immutable expressions should be allowed.  I'm not sure what the
> resolution was, but in any case there should be documentation and tests
> of the outcome.

The partition bound expression is evaluated only once, that is, when the
partition creation command is executed, and what gets stored in the
catalog is a Const expression that contains the value that was computed,
not the original non-immutable expression.  So, we don't need to do
anything special in terms of code to handle users possibly specifying a
non-immutable expression as partition bound.  Tom said something in that
thread which seems to support such a position:

https://www.postgresql.org/message-id/22534.1523374457%40sss.pgh.pa.us

Part of the test case that was removed is the error that used to be emitted:

 CREATE TABLE moneyp (
 a money
 ) PARTITION BY LIST (a);
 CREATE TABLE moneyp_10 PARTITION OF moneyp FOR VALUES IN (10);
-ERROR:  specified value cannot be cast to type money for column "a"
-LINE 1: ...EATE TABLE moneyp_10 PARTITION OF moneyp FOR VALUES IN (10);
-   ^
-DETAIL:  The cast requires a non-immutable conversion.
-HINT:  Try putting the literal value in single quotes.
-CREATE TABLE moneyp_10 PARTITION OF moneyp FOR VALUES IN ('10');

which is no longer emitted, because the patched
transformPartitionBoundValue evaluates the expression, instead of emitting
error because the expression hasn't become a Const even after applying
eval_const_expressions().

Do we need to mention any aspect of how this now works in the user-facing
documentation?

> The collation handling might need another look.  The following is
> allowed without any diagnostic:
> 
> CREATE TABLE t2 (
> a text COLLATE "POSIX"
> ) PARTITION BY RANGE (a);
> CREATE TABLE t2a PARTITION OF t2 FOR VALUES FROM ('foo' COLLATE "C") TO
> ('xyz');
> 
> I think the correct behavior would be that an explicit collation in the
> partition bound expression is an error.

I tend to agree with that.  What happens is the partition bound expression
is assigned the collation of the parent's partition key:

+partcollation = get_partition_col_collation(key, 0);

+value = transformPartitionBoundValue(pstate, expr,
+ colname, coltype, coltypmod,
+ partcollation);

But that's essentially ignoring the collation specified by the user for
the partition bound value without providing any information about that to
the user.  I've fixed that by explicitly checking if the collate clause in
the partition bound value expression contains the same collation as the
parent partition key collation and give an error otherwise.

Updated patch attached.

Thanks,
Amit
>From 4c14559e2d8412c9ec4990e078cdc017a78ca69f Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 6 Jul 2018 14:05:22 +0900
Subject: [PATCH v8] Allow generalized expression syntax for partition bounds

Authors: Kyotaro Horiguchi, Tom Lane, Amit Langote
---
 doc/src/sgml/ref/alter_table.sgml  |   6 +-
 doc/src/sgml/ref/create_table.sgml |  16 +--
 src/backend/commands/tablecmds.c   |   9 ++
 src/backend/optimizer/util/clauses.c   |   4 +-
 src/backend/parser/gram.y  |  60 +---
 src/backend/parser/parse_agg.c |  10 ++
 src/backend/parser/parse_expr.c|   5 +
 src/backend/parser/parse_func.c|   3 +
 src/backend/parser/parse_utilcmd.c | 211 +++--
 src/include/optimizer/clauses.h|   2 +
 src/include/parser/parse_node.h|   1 +
 src/include/utils/partcache.h  |   6 +
 src/test/regress/expected/create_table.out |  69 +++---
 src/test/regress/sql/create_table.sql  |  31 -
 14 files changed, 265 insertions(+), 168 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml 
b/doc/src/sgml/ref/alter_table.sgml
index 0d1feaf828..bd037f2e39 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ 

Re: using expression syntax for partition bounds

[Peter Eisentraut]

On 26/11/2018 05:58, Amit Langote wrote:
> On 2018/11/09 14:38, Amit Langote wrote:
>> Rebased due to change in addRangeTableEntryForRelation's API.
> 
> Rebased again due to changes in psql's describe output for partitioned tables.

Review:

Is "partition bound" the right term?  For list partitioning, it's not
really a bound.  Maybe it's OK.

Keep the ordering of EXPR_KIND_PARTITION_BOUND in the various switch
statements consistent.

I don't see any treatment of non-immutable expressions.  There is a test
case with a non-immutable cast that was removed, but that's all.  There
was considerable discussion earlier in the thread on whether
non-immutable expressions should be allowed.  I'm not sure what the
resolution was, but in any case there should be documentation and tests
of the outcome.

The collation handling might need another look.  The following is
allowed without any diagnostic:

CREATE TABLE t2 (
a text COLLATE "POSIX"
) PARTITION BY RANGE (a);
CREATE TABLE t2a PARTITION OF t2 FOR VALUES FROM ('foo' COLLATE "C") TO
('xyz');

I think the correct behavior would be that an explicit collation in the
partition bound expression is an error.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: using expression syntax for partition bounds

[Amit Langote]

On 2018/11/09 14:38, Amit Langote wrote:
> Rebased due to change in addRangeTableEntryForRelation's API.

Rebased again due to changes in psql's describe output for partitioned tables.

Thanks,
Amit
>From 999aa53b459a6fc0fe899e613406f0e0035aca94 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 6 Jul 2018 14:05:22 +0900
Subject: [PATCH v7] Allow generalized expression syntax for partition bounds

Authors: Kyotaro Horiguchi, Tom Lane, Amit Langote
---
 doc/src/sgml/ref/alter_table.sgml  |   6 +-
 doc/src/sgml/ref/create_table.sgml |  16 +--
 src/backend/commands/tablecmds.c   |   9 ++
 src/backend/optimizer/util/clauses.c   |   4 +-
 src/backend/parser/gram.y  |  60 +
 src/backend/parser/parse_agg.c |  10 ++
 src/backend/parser/parse_expr.c|   5 +
 src/backend/parser/parse_func.c|   3 +
 src/backend/parser/parse_utilcmd.c | 201 ++---
 src/include/optimizer/clauses.h|   2 +
 src/include/parser/parse_node.h|   1 +
 src/include/utils/partcache.h  |   6 +
 src/test/regress/expected/create_table.out |  49 ---
 src/test/regress/sql/create_table.sql  |  16 ++-
 14 files changed, 222 insertions(+), 166 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml 
b/doc/src/sgml/ref/alter_table.sgml
index be1647937d..b9c5f7c384 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -86,9 +86,9 @@ ALTER TABLE [ IF EXISTS ] name
 
 and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | TRUE | FALSE | 
NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 and column_constraint 
is:
diff --git a/doc/src/sgml/ref/create_table.sgml 
b/doc/src/sgml/ref/create_table.sgml
index 50d5597002..7297f751b7 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -86,9 +86,9 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] 
TABLE [ IF NOT EXI
 
 and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | TRUE | FALSE | 
NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 index_parameters in 
UNIQUE, PRIMARY KEY, and 
EXCLUDE constraints are:
@@ -412,12 +412,10 @@ WITH ( MODULUS numeric_literal, REM
  
 
  
-  Each of the values specified in
-  the partition_bound_spec is
-  a literal, NULL, MINVALUE, or
-  MAXVALUE.  Each literal value must be either a
-  numeric constant that is coercible to the corresponding partition key
-  column's type, or a string literal that is valid input for that type.
+  partition_bound_expr is
+  any variable-free expression (subqueries, window functions, aggregate
+  functions, and set-returning functions are not allowed).  Its data type
+  must match the data type of the corresponding partition key column.
  
 
  
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 843ed48aa7..32a094dcf6 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -767,6 +767,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
defaultPartOid;
Relationparent,
defaultRel = NULL;
+   RangeTblEntry *rte;
 
/* Already have strong enough lock on the parent */
parent = heap_open(parentId, NoLock);
@@ -809,6 +810,14 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
pstate = make_parsestate(NULL);
pstate->p_sourcetext = queryString;
 
+   /*
+* Add an RTE containing this relation, so that transformExpr 
called
+* on partition bound expressions is able to report errors 
using a
+* proper context.
+*/
+   rte = addRangeTableEntryForRelation(pstate, rel, 
AccessShareLock,
+   
NULL, false, false);
+   addRTEtoQuery(pstate, rte, false, true, true);

Re: using expression syntax for partition bounds

[Amit Langote]

Rebased due to change in addRangeTableEntryForRelation's API.

Thanks,
Amit
>From 2c9bd7d17abea93001c923ac200c560417cd39a1 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 6 Jul 2018 14:05:22 +0900
Subject: [PATCH v6] Allow generalized expression syntax for partition bounds

Authors: Kyotaro Horiguchi, Tom Lane, Amit Langote
---
 doc/src/sgml/ref/alter_table.sgml  |   6 +-
 doc/src/sgml/ref/create_table.sgml |  16 +--
 src/backend/commands/tablecmds.c   |   9 ++
 src/backend/optimizer/util/clauses.c   |   4 +-
 src/backend/parser/gram.y  |  60 +
 src/backend/parser/parse_agg.c |  10 ++
 src/backend/parser/parse_expr.c|   5 +
 src/backend/parser/parse_func.c|   3 +
 src/backend/parser/parse_utilcmd.c | 201 ++---
 src/include/optimizer/clauses.h|   2 +
 src/include/parser/parse_node.h|   1 +
 src/include/utils/partcache.h  |   6 +
 src/test/regress/expected/create_table.out |  49 ---
 src/test/regress/sql/create_table.sql  |  16 ++-
 14 files changed, 222 insertions(+), 166 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml 
b/doc/src/sgml/ref/alter_table.sgml
index f13a6cd944..7ec9dc4537 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -87,9 +87,9 @@ ALTER TABLE [ IF EXISTS ] name
 
 and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | TRUE | FALSE | 
NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 and column_constraint 
is:
diff --git a/doc/src/sgml/ref/create_table.sgml 
b/doc/src/sgml/ref/create_table.sgml
index 4b9c8a7801..aa60f60e6a 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -86,9 +86,9 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] 
TABLE [ IF NOT EXI
 
 and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | TRUE | FALSE | 
NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | TRUE | FALSE | 
MINVALUE | MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 index_parameters in 
UNIQUE, PRIMARY KEY, and 
EXCLUDE constraints are:
@@ -412,12 +412,10 @@ WITH ( MODULUS numeric_literal, REM
  
 
  
-  Each of the values specified in
-  the partition_bound_spec is
-  a literal, NULL, MINVALUE, or
-  MAXVALUE.  Each literal value must be either a
-  numeric constant that is coercible to the corresponding partition key
-  column's type, or a string literal that is valid input for that type.
+  partition_bound_expr is
+  any variable-free expression (subqueries, window functions, aggregate
+  functions, and set-returning functions are not allowed).  Its data type
+  must match the data type of the corresponding partition key column.
  
 
  
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 82989158ee..421d50bb9f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -797,6 +797,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
defaultPartOid;
Relationparent,
defaultRel = NULL;
+   RangeTblEntry *rte;
 
/* Already have strong enough lock on the parent */
parent = heap_open(parentId, NoLock);
@@ -839,6 +840,14 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
pstate = make_parsestate(NULL);
pstate->p_sourcetext = queryString;
 
+   /*
+* Add an RTE containing this relation, so that transformExpr 
called
+* on partition bound expressions is able to report errors 
using a
+* proper context.
+*/
+   rte = addRangeTableEntryForRelation(pstate, rel, 
AccessShareLock,
+   
NULL, false, false);
+   addRTEtoQuery(pstate, rte, false, true, true);
bound = transformPartitionBound(pstate, parent, 
stmt->partbound);
 
/*
diff --git 

Re: using expression syntax for partition bounds

[Amit Langote]

Horiguchi-san,

On 2018/07/06 14:26, Kyotaro HORIGUCHI wrote:
> Hello.
> 
> cf-bot compained on this but I wondered why it got so many
> errors. At the first look I found a spare semicolon before a bare
> then calause:p
> 
>> -if (!IsA(value, Const));
>> +if (!IsA(value, Const))
>>  elog(ERROR, "could not evaluate partition bound expression");
> 
> The attached is the fixed and rebsaed version.

Thanks for taking care of that.

Regards,
Amit

Re: using expression syntax for partition bounds

[Kyotaro HORIGUCHI]

Hello.

cf-bot compained on this but I wondered why it got so many
errors. At the first look I found a spare semicolon before a bare
then calause:p

> - if (!IsA(value, Const));
> + if (!IsA(value, Const))
>   elog(ERROR, "could not evaluate partition bound expression");

The attached is the fixed and rebsaed version.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From bbf76ee4a185133e625be88bf0784f2bc308772b Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 6 Jul 2018 14:05:22 +0900
Subject: [PATCH] Allow generalized expression syntax for partition bounds

Authors: Kyotaro Horiguchi, Tom Lane, Amit Langote
---
 doc/src/sgml/ref/alter_table.sgml  |   6 +-
 doc/src/sgml/ref/create_table.sgml |  16 +--
 src/backend/commands/tablecmds.c   |   8 ++
 src/backend/optimizer/util/clauses.c   |   4 +-
 src/backend/parser/gram.y  |  60 +
 src/backend/parser/parse_agg.c |  10 ++
 src/backend/parser/parse_expr.c|   5 +
 src/backend/parser/parse_func.c|   3 +
 src/backend/parser/parse_utilcmd.c | 201 ++---
 src/include/optimizer/clauses.h|   2 +
 src/include/parser/parse_node.h|   1 +
 src/include/utils/partcache.h  |   6 +
 src/test/regress/expected/create_table.out |  49 ---
 src/test/regress/sql/create_table.sql  |  16 ++-
 14 files changed, 221 insertions(+), 166 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 1cce00eaf9..aa70beeb32 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -87,9 +87,9 @@ ALTER TABLE [ IF EXISTS ] name
 
 and partition_bound_spec is:
 
-IN ( { numeric_literal | string_literal | TRUE | FALSE | NULL } [, ...] ) |
-FROM ( { numeric_literal | string_literal | TRUE | FALSE | MINVALUE | MAXVALUE } [, ...] )
-  TO ( { numeric_literal | string_literal | TRUE | FALSE | MINVALUE | MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] ) |
+FROM ( { partition_bound_expr | MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, REMAINDER numeric_literal )
 
 and column_constraint is:
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 2a1eac9592..fcbabc78ff 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -86,9 +86,9 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI
 
 and partition_bound_spec is:
 
-IN ( { numeric_literal | string_literal | TRUE | FALSE | NULL } [, ...] ) |
-FROM ( { numeric_literal | string_literal | TRUE | FALSE | MINVALUE | MAXVALUE } [, ...] )
-  TO ( { numeric_literal | string_literal | TRUE | FALSE | MINVALUE | MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] ) |
+FROM ( { partition_bound_expr | MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, REMAINDER numeric_literal )
 
 index_parameters in UNIQUE, PRIMARY KEY, and EXCLUDE constraints are:
@@ -273,12 +273,10 @@ WITH ( MODULUS numeric_literal, REM
  
 
  
-  Each of the values specified in
-  the partition_bound_spec is
-  a literal, NULL, MINVALUE, or
-  MAXVALUE.  Each literal value must be either a
-  numeric constant that is coercible to the corresponding partition key
-  column's type, or a string literal that is valid input for that type.
+  partition_bound_expr is
+  any variable-free expression (subqueries, window functions, aggregate
+  functions, and set-returning functions are not allowed).  Its data type
+  must match the data type of the corresponding partition key column.
  
 
  
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7c0cf0d7ee..fc24e6fa2b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -796,6 +796,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 	defaultPartOid;
 		Relation	parent,
 	defaultRel = NULL;
+		RangeTblEntry *rte;
 
 		/* Already have strong enough lock on the parent */
 		parent = heap_open(parentId, NoLock);
@@ -838,6 +839,13 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		pstate = make_parsestate(NULL);
 		pstate->p_sourcetext = queryString;
 
+		/*
+		 * Add an RTE containing this relation, so that transformExpr called
+		 * on partition bound expressions is able to report errors using a
+		 * proper context.
+		 */
+		rte = addRangeTableEntryForRelation(pstate, rel, NULL, false, false);
+		addRTEtoQuery(pstate, rte, false, true, true);
 		bound = transformPartitionBound(pstate, parent, stmt->partbound);
 
 		/*
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 505ae0af85..77ebb40ef2 

Re: using expression syntax for partition bounds

[Amit Langote]

Horiguchi-san,

Thanks a lot for the review and sorry it took me a while to reply.
Thought I'd refresh the patch as it's in the July CF.

On 2018/04/24 18:08, Kyotaro HORIGUCHI wrote:
> Thanks. I have almost missed this.
> 
> At Mon, 23 Apr 2018 11:44:14 +0900, Amit Langote wrote:
>> On 2018/04/23 11:37, Amit Langote wrote:
>>> I tried to update the patch to do things that way.  I'm going to create a
>>> new entry in the next CF titled "generalized expression syntax for
>>> partition bounds" and add the patch there.
>>
>> Tweaked the commit message to credit all the authors.
> 
> +  any variable-free expression (subqueries, window functions, aggregate
> +  functions, and set-returning functions are not allowed).  The data type
> +  of the partition bound expression must match the data type of the
> +  corresponding partition key column.
> 
> Parititioning value is slimiar to column default expression in
> the sense that it appeas within a DDL. The description for
> DEFAULT is:
> 
> |  The value is any variable-free expression (subqueries and
> |  cross-references to other columns in the current table are not
> |  allowed)
> 
> It actually refuses aggregates, window functions and SRFs but it
> is not mentioned.  Whichever we choose, they ought to have the
> similar description.

Actually, I referenced the default value expression syntax a lot when
working on this issue, so agree that there's a close resemblance.

Maybe, we should fix the description for default expression to say that it
can't contain subqueries, cross-references to other columns in the table,
aggregate expressions, window functions, and set-returning functions.  I
also sent a patch separately:

https://www.postgresql.org/message-id/2059e8f2-e6e6-7a9f-0de8-11eed291e...@lab.ntt.co.jp

>>   /*
>>* Strip any top-level COLLATE clause, as they're inconsequential.
>>* XXX - Should we add a WARNING here?
>>*/
>>   while (IsA(value, CollateExpr))
>>   value = (Node *) ((CollateExpr *) value)->arg;
> 
> Ok, I'll follow Tom's suggestion but collate is necessarilly
> appears in this shape. For example ('a' collate "de_DE") || 'b')
> has the collate node under the top ExprOp and we get a complaint
> like following with it.
> 
>> ERROR:  unrecognized node type: 123
> 
> 123 is T_CollateExpr. The expression "value" (mmm..) reuires
> eval_const_expressions to eliminate CollateExprs.  It requires
> assign_expr_collations to retrieve value's collation but we don't
> do that since we ignore collation this in this case.

Ah yes, it seems better to call eval_const_expressions as a first step to
get rid of CollateExpr's, followed by evaluate_expr if the first step
didn't return a Const node.

> The following results in a strange-looking error.
> 
>> =# create table pt1 partition of p for values in (a);
>> ERROR:  column "a" does not exist
>> LINE 1: create table pt1 partition of p for values in (a);
> 
> The p/pt1 actually have a column a.
> 
> The following results in similar error and it is wrong, too.
> 
>> =# create table pr1 partition of pr for values from (a + 1) to (a + 2);
>> ERROR: column "a" does not exist
>> LINE 1: create table pr1 partition of pr for values from (a + 1) to ...

This one is better fixed by initializing ParseState properly as
demonstrated in your v3-2 patch.

> Being similar but a bit different, the following command leads to
> a SEGV since it creates PARTITION_RANGE_DATUM_VALUE with value =
> NULL. Even if it leaves the original node, validateInfiniteBounds
> rejects it and aborts.
> 
>> =# create table pr1 partition of pr for values from (hoge) to (hige);
> (crash)

Oops.

> I fixed this using pre-columnref hook in the attached v3. This
> behavles for columnrefs differently than DEFAULT.
> 
> The v3-2 does the same thing with DEFAULT. The behevior differs
> whether the column exists or not.
> 
>> ERROR:  cannot use column referencees in bound value
>> ERROR:  column "b" does not exist
> 
> I personally think that such similarity between DEFALUT and
> partition bound (v3-2) is not required. But inserting the hook
> (v3) doesn't look good for me.

Actually, I too like the solution of v3-2 better, instead of using the
hook, so I adopted it in the updated patch.

I also changed how range bounds are processed in transformPartitionBound,
moving some code into newly added transformPartitionRangeBounds to reduce
code duplication.

Updated patch attached.

Thanks,
Amit
>From 81aacd6f66fd935aa1df6997b3678726e3d951ee Mon Sep 17 00:00:00 2001
From: amit 
Date: Wed, 18 Apr 2018 18:53:44 +0900
Subject: [PATCH v4] Allow generalized expression syntax for partition bounds

Authors: Kyotaro Horiguchi, Tom Lane, Amit Langote
---
 doc/src/sgml/ref/alter_table.sgml  |   6 +-
 doc/src/sgml/ref/create_table.sgml |  16 +--
 src/backend/commands/tablecmds.c   |   8 ++
 src/backend/optimizer/util/clauses.c   |   4 +-
 src/backend/parser/gram.y  |  60 +
 

Re: using expression syntax for partition bounds

[Kyotaro HORIGUCHI]

Thanks. I have almost missed this.

At Mon, 23 Apr 2018 11:44:14 +0900, Amit Langote 
 wrote in 

> On 2018/04/23 11:37, Amit Langote wrote:
> > I tried to update the patch to do things that way.  I'm going to create a
> > new entry in the next CF titled "generalized expression syntax for
> > partition bounds" and add the patch there.
> 
> Tweaked the commit message to credit all the authors.

+  any variable-free expression (subqueries, window functions, aggregate
+  functions, and set-returning functions are not allowed).  The data type
+  of the partition bound expression must match the data type of the
+  corresponding partition key column.

Parititioning value is slimiar to column default expression in
the sense that it appeas within a DDL. The description for
DEFAULT is:

|  The value is any variable-free expression (subqueries and
|  cross-references to other columns in the current table are not
|  allowed)

It actually refuses aggregates, window functions and SRFs but it
is not mentioned.  Whichever we choose, they ought to have the
similar description.

>   /*
>* Strip any top-level COLLATE clause, as they're inconsequential.
>* XXX - Should we add a WARNING here?
>*/
>   while (IsA(value, CollateExpr))
>   value = (Node *) ((CollateExpr *) value)->arg;

Ok, I'll follow Tom's suggestion but collate is necessarilly
appears in this shape. For example ('a' collate "de_DE") || 'b')
has the collate node under the top ExprOp and we get a complaint
like following with it.

> ERROR:  unrecognized node type: 123

123 is T_CollateExpr. The expression "value" (mmm..) reuires
eval_const_expressions to eliminate CollateExprs.  It requires
assign_expr_collations to retrieve value's collation but we don't
do that since we ignore collation this in this case.


The following results in a strange-looking error.

> =# create table pt1 partition of p for values in (a);
> ERROR:  column "a" does not exist
> LINE 1: create table pt1 partition of p for values in (a);

The p/pt1 actually have a column a.

The following results in similar error and it is wrong, too.

> =# create table pr1 partition of pr for values from (a + 1) to (a + 2);
> ERROR: column "a" does not exist
> LINE 1: create table pr1 partition of pr for values from (a + 1) to ...

Being similar but a bit different, the following command leads to
a SEGV since it creates PARTITION_RANGE_DATUM_VALUE with value =
NULL. Even if it leaves the original node, validateInfiniteBounds
rejects it and aborts.

> =# create table pr1 partition of pr for values from (hoge) to (hige);
(crash)


I fixed this using pre-columnref hook in the attached v3. This
behavles for columnrefs differently than DEFAULT.

The v3-2 does the same thing with DEFAULT. The behevior differs
whether the column exists or not.

> ERROR:  cannot use column referencees in bound value
> ERROR:  column "b" does not exist

I personally think that such similarity between DEFALUT and
partition bound (v3-2) is not required. But inserting the hook
(v3) doesn't look good for me.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 763b4f573c..b6f3ddc22f 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -275,10 +275,10 @@ WITH ( MODULUS numeric_literal, REM
  
   Each of the values specified in
   the partition_bound_spec is
-  a literal, NULL, MINVALUE, or
-  MAXVALUE.  Each literal value must be either a
-  numeric constant that is coercible to the corresponding partition key
-  column's type, or a string literal that is valid input for that type.
+  any variable-free expression (subqueries and cross-references to other
+  columns in the current table are not allowed).  The data type of the
+  partition bound expression must match the data type of the corresponding
+  partition key column.
  
 
  
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 505ae0af85..77ebb40ef2 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -150,8 +150,6 @@ static Node *substitute_actual_parameters(Node *expr, int nargs, List *args,
 static Node *substitute_actual_parameters_mutator(Node *node,
 	 substitute_actual_parameters_context *context);
 static void sql_inline_error_callback(void *arg);
-static Expr *evaluate_expr(Expr *expr, Oid result_type, int32 result_typmod,
-			  Oid result_collation);
 static Query *substitute_actual_srf_parameters(Query *expr,
  int nargs, List *args);
 static Node *substitute_actual_srf_parameters_mutator(Node *node,
@@ -4840,7 +4838,7 @@ sql_inline_error_callback(void *arg)
  * We use the executor's routine ExecEvalExpr() to avoid duplication of
  * code and ensure we get the same 

Re: using expression syntax for partition bounds (was: Re: Boolean partitions syntax)

[Amit Langote]

On 2018/04/23 11:37, Amit Langote wrote:
> I tried to update the patch to do things that way.  I'm going to create a
> new entry in the next CF titled "generalized expression syntax for
> partition bounds" and add the patch there.

Tweaked the commit message to credit all the authors.

Thanks,
Amit
From c3b78623fdd37c785eb676484945d15448357591 Mon Sep 17 00:00:00 2001
From: amit 
Date: Wed, 18 Apr 2018 18:53:44 +0900
Subject: [PATCH v2] Allow generalized expression syntax for partition bounds

This fixes the bug that Boolean literals true/false are not allowed
in partition bound syntax.

Authors: Kyotaro Horiguchi, Tom Lane, Amit Langote
---
 doc/src/sgml/ref/alter_table.sgml  |   6 +-
 doc/src/sgml/ref/create_table.sgml |  14 ++--
 src/backend/optimizer/util/clauses.c   |   4 +-
 src/backend/parser/gram.y  |  58 +
 src/backend/parser/parse_agg.c |  10 +++
 src/backend/parser/parse_expr.c|   5 ++
 src/backend/parser/parse_func.c|   3 +
 src/backend/parser/parse_utilcmd.c | 126 ++---
 src/include/optimizer/clauses.h|   2 +
 src/include/parser/parse_node.h|   1 +
 src/include/utils/partcache.h  |   6 ++
 src/test/regress/expected/create_table.out |  16 +---
 src/test/regress/sql/create_table.sql  |   5 +-
 13 files changed, 142 insertions(+), 114 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml 
b/doc/src/sgml/ref/alter_table.sgml
index 8c7b9619b8..2c11a5ad75 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -87,9 +87,9 @@ ALTER TABLE [ IF EXISTS ] name
 
 and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | MINVALUE | 
MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | MINVALUE | 
MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 and column_constraint 
is:
diff --git a/doc/src/sgml/ref/create_table.sgml 
b/doc/src/sgml/ref/create_table.sgml
index f2bd562d55..b7d5562345 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -86,9 +86,9 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] 
TABLE [ IF NOT EXI
 
 and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | MINVALUE | 
MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | MINVALUE | 
MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 index_parameters in 
UNIQUE, PRIMARY KEY, and 
EXCLUDE constraints are:
@@ -275,10 +275,10 @@ WITH ( MODULUS numeric_literal, REM
  
   Each of the values specified in
   the partition_bound_spec is
-  a literal, NULL, MINVALUE, or
-  MAXVALUE.  Each literal value must be either a
-  numeric constant that is coercible to the corresponding partition key
-  column's type, or a string literal that is valid input for that type.
+  any variable-free expression (subqueries, window functions, aggregate
+  functions, and set-returning functions are not allowed).  The data type
+  of the partition bound expression must match the data type of the
+  corresponding partition key column.
  
 
  
diff --git a/src/backend/optimizer/util/clauses.c 
b/src/backend/optimizer/util/clauses.c
index 505ae0af85..77ebb40ef2 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -150,8 +150,6 @@ static Node *substitute_actual_parameters(Node *expr, int 
nargs, List *args,
 static Node *substitute_actual_parameters_mutator(Node *node,
 
substitute_actual_parameters_context *context);
 static void sql_inline_error_callback(void *arg);
-static Expr *evaluate_expr(Expr *expr, Oid result_type, int32 result_typmod,
- Oid result_collation);
 static Query *substitute_actual_srf_parameters(Query *expr,
 int nargs, 
List *args);
 static Node *substitute_actual_srf_parameters_mutator(Node *node,
@@ -4840,7 +4838,7 @@ sql_inline_error_callback(void *arg)
  * We use the executor's routine ExecEvalExpr() to avoid duplication of
  * code and ensure we get the same result as the executor would get.
  */
-static Expr *
+Expr *
 evaluate_expr(Expr *expr, Oid result_type, int32 result_typmod,
  Oid 

using expression syntax for partition bounds (was: Re: Boolean partitions syntax)

[Amit Langote]

(patch and discussion for PG 12)

On 2018/04/22 1:28, Tom Lane wrote:
> Amit Langote  writes:
>> [ v8-0001-Allow-generalized-expression-syntax-for-partition.patch ]
>
> I find what you did to a_expr here to be pretty horrid.

Thanks for the review.

> I think what you should do is lose the partbound_datum and
> PartitionRangeDatum productions altogether, replacing those with just
> a_expr, as in the attached grammar-only patch.  This would result in
> needing to identify MINVALUE and MAXVALUE during parse analysis, since
> the grammar would just treat them as ColId expressions.  But since we're
> not intending to ever allow any actual column references in partition
> expressions, I don't see any harm in allowing parse analysis to treat
> ColumnRefs containing those names as meaning the special items.

I have to agree this is better.

> This is a little bit grotty, in that both MINVALUE and "minvalue" would
> be recognized as the keyword, but it's sure a lot less messy than what's
> there now.  And IIRC there are some other places where we're a bit
> squishy about the difference between identifiers and keywords, too.

Hmm, yes.

I tried to update the patch to do things that way.  I'm going to create a
new entry in the next CF titled "generalized expression syntax for
partition bounds" and add the patch there.

Thanks,
Amit
From e82738affc8f3e4c52b1ee6dd6a54cf23b991d6c Mon Sep 17 00:00:00 2001
From: amit 
Date: Wed, 18 Apr 2018 18:53:44 +0900
Subject: [PATCH v1] Allow generalized expression syntax for partition bounds

This fixes the bug that Boolean literals true/false are not allowed
in partition bound syntax.

Authors: Kyotaro Horiguchi, Amit Langote
---
 doc/src/sgml/ref/alter_table.sgml  |   6 +-
 doc/src/sgml/ref/create_table.sgml |  14 ++--
 src/backend/optimizer/util/clauses.c   |   4 +-
 src/backend/parser/gram.y  |  58 +
 src/backend/parser/parse_agg.c |  10 +++
 src/backend/parser/parse_expr.c|   5 ++
 src/backend/parser/parse_func.c|   3 +
 src/backend/parser/parse_utilcmd.c | 126 ++---
 src/include/optimizer/clauses.h|   2 +
 src/include/parser/parse_node.h|   1 +
 src/include/utils/partcache.h  |   6 ++
 src/test/regress/expected/create_table.out |  16 +---
 src/test/regress/sql/create_table.sql  |   5 +-
 13 files changed, 142 insertions(+), 114 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml 
b/doc/src/sgml/ref/alter_table.sgml
index 8c7b9619b8..2c11a5ad75 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -87,9 +87,9 @@ ALTER TABLE [ IF EXISTS ] name
 
 and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | MINVALUE | 
MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | MINVALUE | 
MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 and column_constraint 
is:
diff --git a/doc/src/sgml/ref/create_table.sgml 
b/doc/src/sgml/ref/create_table.sgml
index f2bd562d55..b7d5562345 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -86,9 +86,9 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] 
TABLE [ IF NOT EXI
 
 and partition_bound_spec 
is:
 
-IN ( { numeric_literal | 
string_literal | NULL } [, ...] ) |
-FROM ( { numeric_literal | 
string_literal | MINVALUE | 
MAXVALUE } [, ...] )
-  TO ( { numeric_literal | 
string_literal | MINVALUE | 
MAXVALUE } [, ...] ) |
+IN ( partition_bound_expr [, ...] 
) |
+FROM ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] )
+  TO ( { partition_bound_expr | 
MINVALUE | MAXVALUE } [, ...] ) |
 WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
 index_parameters in 
UNIQUE, PRIMARY KEY, and 
EXCLUDE constraints are:
@@ -275,10 +275,10 @@ WITH ( MODULUS numeric_literal, REM
  
   Each of the values specified in
   the partition_bound_spec is
-  a literal, NULL, MINVALUE, or
-  MAXVALUE.  Each literal value must be either a
-  numeric constant that is coercible to the corresponding partition key
-  column's type, or a string literal that is valid input for that type.
+  any variable-free expression (subqueries, window functions, aggregate
+  functions, and set-returning functions are not allowed).  The data type
+  of the partition bound expression must match the data type of the
+  corresponding partition key column.
  
 
  
diff --git a/src/backend/optimizer/util/clauses.c 
b/src/backend/optimizer/util/clauses.c
index 505ae0af85..77ebb40ef2 100644
---