Re: Boolean partitions syntax - Mailing list pgsql-hackers

At Wed, 11 Apr 2018 11:27:17 +0900, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote in
<1810b14f-3cd7-aff5-8358-c225c0231ee9@lab.ntt.co.jp>
> On 2018/04/11 10:44, Tom Lane wrote:
> > Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
> >> At least partition bound *must* be a constant. Any expression
> >> that can be reduced to a constant at parse time ought to be
> >> accepted but must not be accepted if not.
> > 
> > My point is that *any* expression can be reduced to a constant,
> > we just have to do so.

Agreed in that sense. What was in my mind was something like
column reference, random() falls into reducible category of
course.

# I regard the change in gram.y is regarded as acceptable as a
# direction, so I'll continue to working on this.

> Currently transformPartitionBoundValue() applies eval_const_expressions()
> by way of calling expression_planner().  However passing to it, say, an
> expression representing random() is unable to reduce it to a Const because
> simplify_function/evaluate_function won't compute a mutable function like
> random().  So, that currently results in an error like this (note that
> this is after applying Horiguchi-san's latest patch that enables
> specifying random() as a partition bound in the first place):
> 
> create table foo_part partition of foo for values in ((random())::int);
> ERROR:  specified value cannot be cast to type integer for column "a"
> LINE 1: ...table foo_random partition of foo for values in ((random()):...
>                                                              ^
> DETAIL:  The cast requires a non-immutable conversion.
> HINT:  Try putting the literal value in single quotes.
> 
> The error is output after the following if check in
> transformPartitionBoundValue fails:
> 
>     /* Fail if we don't have a constant (i.e., non-immutable coercion) */
>     if (!IsA(value, Const))
> 
> I think what Tom is proposing here, instead of bailing out upon
> eval_const_expressions() failing to simplify the input expression to a
> Const, is to *invoke the executor* to evaluate the expression, like the
> optimizer does in evaluate_expr, and cook up a Const with whatever comes
> out to store it into the catalog (that is, in relpartbound).

Yes. In the attached I used evaluate_expr by making it non-static
function. a_expr used instead of partbound_datum is changed to
u_expr, which is the same with range bounds.

> =# create table c1 partition of p for values in (random() * 100);
> CREATE TABLE
> =# \d c1
...
> Partition of: p FOR VALUES IN (97)

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index ed6b680ed8..cfb0984100 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -152,8 +152,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,
@@ -4842,7 +4840,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 result_collation)
 {
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index dd0c26c11b..02979a3533 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -180,6 +180,8 @@ static Node *makeXmlExpr(XmlExprOp op, char *name, List *named_args,
 static List *mergeTableFuncParameters(List *func_args, List *columns);
 static TypeName *TableFuncTypeName(List *columns);
 static RangeVar *makeRangeVarFromAnyName(List *names, int position, core_yyscan_t yyscanner);
+static Node *makePartRangeDatum(PartitionRangeDatumKind kind, Node *value,
+                                int location);
 static void SplitColQualList(List *qualList,
                              List **constraintList, CollateClause **collClause,
                              core_yyscan_t yyscanner);
@@ -472,7 +474,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <node>    columnDef columnOptions
 %type <defelt>    def_elem reloption_elem old_aggr_elem operator_def_elem
 %type <node>    def_arg columnElem where_clause where_or_current_clause
-                a_expr b_expr c_expr AexprConst indirection_el opt_slice_bound
+                a_expr u_expr b_expr b0_expr c_expr c0_expr AexprConst indirection_el opt_slice_bound
                 columnref in_expr having_clause func_table xmltable array_expr
                 ExclusionWhereClause operator_def_arg
 %type <list>    rowsfrom_item rowsfrom_list opt_col_def_list
@@ -585,7 +587,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <partelem>    part_elem
 %type <list>        part_params
 %type <partboundspec> PartitionBoundSpec
-%type <node>        partbound_datum PartitionRangeDatum
+%type <node>        PartitionRangeDatum
 %type <list>        hash_partbound partbound_datum_list range_datum_list
 %type <defelt>        hash_partbound_elem
 
@@ -2804,15 +2806,9 @@ hash_partbound:
             }
         ;
 
-partbound_datum:
-            Sconst            { $$ = makeStringConst($1, @1); }
-            | NumericOnly    { $$ = makeAConst($1, @1); }
-            | NULL_P        { $$ = makeNullAConst(@1); }
-        ;
-
 partbound_datum_list:
-            partbound_datum                        { $$ = list_make1($1); }
-            | partbound_datum_list ',' partbound_datum
+            u_expr                        { $$ = list_make1($1); }
+            | partbound_datum_list ',' a_expr
                                                 { $$ = lappend($1, $3); }
         ;
 
@@ -2825,33 +2821,18 @@ range_datum_list:
 PartitionRangeDatum:
             MINVALUE
                 {
-                    PartitionRangeDatum *n = makeNode(PartitionRangeDatum);
-
-                    n->kind = PARTITION_RANGE_DATUM_MINVALUE;
-                    n->value = NULL;
-                    n->location = @1;
-
-                    $$ = (Node *) n;
+                    $$ = makePartRangeDatum(PARTITION_RANGE_DATUM_MINVALUE,
+                                            NULL, @1);
                 }
             | MAXVALUE
                 {
-                    PartitionRangeDatum *n = makeNode(PartitionRangeDatum);
-
-                    n->kind = PARTITION_RANGE_DATUM_MAXVALUE;
-                    n->value = NULL;
-                    n->location = @1;
-
-                    $$ = (Node *) n;
+                    $$ = makePartRangeDatum(PARTITION_RANGE_DATUM_MAXVALUE,
+                                            NULL, @1);
                 }
-            | partbound_datum
+            | u_expr
                 {
-                    PartitionRangeDatum *n = makeNode(PartitionRangeDatum);
-
-                    n->kind = PARTITION_RANGE_DATUM_VALUE;
-                    n->value = $1;
-                    n->location = @1;
-
-                    $$ = (Node *) n;
+                    $$ = makePartRangeDatum(PARTITION_RANGE_DATUM_VALUE,
+                                            $1, @1);
                 }
         ;
 
@@ -13478,9 +13459,20 @@ a_expr:        c_expr                                    { $$ = $1; }
  * cause trouble in the places where b_expr is used.  For simplicity, we
  * just eliminate all the boolean-keyword-operator productions from b_expr.
  */
-b_expr:        c_expr
-                { $$ = $1; }
-            | b_expr TYPECAST Typename
+b_expr:        c_expr { $$ = $1; }
+            | b0_expr { $$ = $1; }
+        ;
+
+/*
+ * u_expr is a subset of b_expr so as to be usable along with
+ * unreserved keywords.
+ */
+u_expr:        c0_expr { $$ = $1; }
+            | b0_expr { $$ = $1; }
+        ;
+
+/* common part of b_expr and u_expr */
+b0_expr: b_expr TYPECAST Typename
                 { $$ = makeTypeCast($1, $3, @2); }
             | '+' b_expr                    %prec UMINUS
                 { $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "+", NULL, $2, @1); }
@@ -13554,7 +13546,11 @@ b_expr:        c_expr
  * ambiguity to the b_expr syntax.
  */
 c_expr:        columnref                                { $$ = $1; }
-            | AexprConst                            { $$ = $1; }
+            | c0_expr                                { $$ = $1; }
+        ;
+
+/* common part of c_expr and u_expr */
+c0_expr:         AexprConst                            { $$ = $1; }
             | PARAM opt_indirection
                 {
                     ParamRef *p = makeNode(ParamRef);
@@ -16275,6 +16271,18 @@ makeRangeVarFromAnyName(List *names, int position, core_yyscan_t yyscanner)
     return r;
 }
 
+static Node *
+makePartRangeDatum(PartitionRangeDatumKind kind, Node *value, int location)
+{
+    PartitionRangeDatum *n = makeNode(PartitionRangeDatum);
+
+    n->kind = kind;
+    n->value = value;
+    n->location = location;
+
+    return (Node *) n;
+}
+
 /* Separate Constraint nodes from COLLATE clauses in a ColQualList */
 static void
 SplitColQualList(List *qualList,
diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 0307738946..61f59f7527 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -506,6 +506,12 @@ check_agglevels_and_constraints(ParseState *pstate, Node *expr)
             else
                 err = _("grouping operations are not allowed in EXECUTE parameters");
 
+        case EXPR_KIND_PARTITION_BOUNDS:
+            if (isAgg)
+                err = _("aggregate functions are not allowed in partition bounds");
+            else
+                err = _("grouping operations are not allowed in partition bounds");
+
             break;
         case EXPR_KIND_TRIGGER_WHEN:
             if (isAgg)
@@ -908,6 +914,8 @@ transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc,
             break;
         case EXPR_KIND_PARTITION_EXPRESSION:
             err = _("window functions are not allowed in partition key expressions");
+        case EXPR_KIND_PARTITION_BOUNDS:
+            err = _("window functions are not allowed in partition bounds");
             break;
         case EXPR_KIND_CALL_ARGUMENT:
             err = _("window functions are not allowed in CALL arguments");
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 38fbe3366f..a7f3d86f75 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -1850,6 +1850,9 @@ transformSubLink(ParseState *pstate, SubLink *sublink)
         case EXPR_KIND_CALL_ARGUMENT:
             err = _("cannot use subquery in CALL argument");
             break;
+        case EXPR_KIND_PARTITION_BOUNDS:
+            err = _("cannot use subquery in partition bounds");
+            break;
 
             /*
              * There is intentionally no default: case here, so that the
@@ -3474,6 +3477,8 @@ ParseExprKindName(ParseExprKind exprKind)
             return "WHEN";
         case EXPR_KIND_PARTITION_EXPRESSION:
             return "PARTITION BY";
+        case EXPR_KIND_PARTITION_BOUNDS:
+            return "partition bounds";
         case EXPR_KIND_CALL_ARGUMENT:
             return "CALL";
         case EXPR_KIND_MERGE_WHEN_AND:
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 615aee6d15..a39e26dd76 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -2306,6 +2306,9 @@ check_srf_call_placement(ParseState *pstate, Node *last_srf, int location)
         case EXPR_KIND_PARTITION_EXPRESSION:
             err = _("set-returning functions are not allowed in partition key expressions");
             break;
+        case EXPR_KIND_PARTITION_BOUNDS:
+            err = _("set-returning functions are not allowed in partition bounds");
+            break;
         case EXPR_KIND_CALL_ARGUMENT:
             err = _("set-returning functions are not allowed in CALL arguments");
             break;
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index f9f9904bad..05b24e7f69 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -48,6 +48,7 @@
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
+#include "optimizer/clauses.h"
 #include "optimizer/planner.h"
 #include "parser/analyze.h"
 #include "parser/parse_clause.h"
@@ -138,7 +139,7 @@ static void transformColumnType(CreateStmtContext *cxt, ColumnDef *column);
 static void setSchemaName(char *context_schema, char **stmt_schema_name);
 static void transformPartitionCmd(CreateStmtContext *cxt, PartitionCmd *cmd);
 static void validateInfiniteBounds(ParseState *pstate, List *blist);
-static Const *transformPartitionBoundValue(ParseState *pstate, A_Const *con,
+static Const *transformPartitionBoundValue(ParseState *pstate, Node *con,
                              const char *colName, Oid colType, int32 colTypmod);
 
 
@@ -3674,12 +3675,12 @@ transformPartitionBound(ParseState *pstate, Relation parent,
         result_spec->listdatums = NIL;
         foreach(cell, spec->listdatums)
         {
-            A_Const    *con = castNode(A_Const, lfirst(cell));
+            Node       *expr = (Node *)lfirst (cell);
             Const       *value;
             ListCell   *cell2;
             bool        duplicate;
 
-            value = transformPartitionBoundValue(pstate, con,
+            value = transformPartitionBoundValue(pstate, expr,
                                                  colname, coltype, coltypmod);
 
             /* Don't add to the result if the value is a duplicate */
@@ -3740,7 +3741,6 @@ transformPartitionBound(ParseState *pstate, Relation parent,
             char       *colname;
             Oid            coltype;
             int32        coltypmod;
-            A_Const    *con;
             Const       *value;
 
             /* Get the column's name in case we need to output an error */
@@ -3761,8 +3761,8 @@ transformPartitionBound(ParseState *pstate, Relation parent,
 
             if (ldatum->value)
             {
-                con = castNode(A_Const, ldatum->value);
-                value = transformPartitionBoundValue(pstate, con,
+                value = transformPartitionBoundValue(pstate,
+                                                     ldatum->value,
                                                      colname,
                                                      coltype, coltypmod);
                 if (value->constisnull)
@@ -3775,8 +3775,8 @@ transformPartitionBound(ParseState *pstate, Relation parent,
 
             if (rdatum->value)
             {
-                con = castNode(A_Const, rdatum->value);
-                value = transformPartitionBoundValue(pstate, con,
+                value = transformPartitionBoundValue(pstate,
+                                                     rdatum->value,
                                                      colname,
                                                      coltype, coltypmod);
                 if (value->constisnull)
@@ -3845,13 +3845,13 @@ validateInfiniteBounds(ParseState *pstate, List *blist)
  * Transform one constant in a partition bound spec
  */
 static Const *
-transformPartitionBoundValue(ParseState *pstate, A_Const *con,
+transformPartitionBoundValue(ParseState *pstate, Node *val,
                              const char *colName, Oid colType, int32 colTypmod)
 {
     Node       *value;
 
     /* Make it into a Const */
-    value = (Node *) make_const(pstate, &con->val, con->location);
+    value = transformExpr(pstate, val, EXPR_KIND_PARTITION_BOUNDS);
 
     /* Coerce to correct type */
     value = coerce_to_target_type(pstate,
@@ -3867,21 +3867,28 @@ transformPartitionBoundValue(ParseState *pstate, A_Const *con,
                 (errcode(ERRCODE_DATATYPE_MISMATCH),
                  errmsg("specified value cannot be cast to type %s for column \"%s\"",
                         format_type_be(colType), colName),
-                 parser_errposition(pstate, con->location)));
+                 parser_errposition(pstate, exprLocation(val))));
 
     /* Simplify the expression, in case we had a coercion */
     if (!IsA(value, Const))
         value = (Node *) expression_planner((Expr *) value);
 
-    /* Fail if we don't have a constant (i.e., non-immutable coercion) */
+    /* Eval if we still don't have a constant (i.e., non-immutable coercion) */
     if (!IsA(value, Const))
-        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)));
+    {
+        Oid        result_type;
+        int32    result_typmod;
+        Oid        result_col;
+
+        /* Collect result characteristics */
+        result_type = exprType(value);
+        result_typmod = exprTypmod(value);
+        result_col = exprCollation(value);
+
+        value = (Node *)evaluate_expr((Expr *) value, result_type,
+                                      result_typmod, result_col);
+        Assert(IsA(value, Const));
+    }
 
     return (Const *) value;
 }
diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h
index ba4fa4b68b..4b1a5b96f8 100644
--- a/src/include/optimizer/clauses.h
+++ b/src/include/optimizer/clauses.h
@@ -85,4 +85,6 @@ extern Node *estimate_expression_value(PlannerInfo *root, Node *node);
 extern Query *inline_set_returning_function(PlannerInfo *root,
                               RangeTblEntry *rte);
 
+extern Expr *evaluate_expr(Expr *expr, Oid result_type, int32 result_typmod,
+                           Oid result_collation);
 #endif                            /* CLAUSES_H */
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 3fd2151ccb..5eccf5078a 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -70,6 +70,7 @@ typedef enum ParseExprKind
     EXPR_KIND_TRIGGER_WHEN,        /* WHEN condition in CREATE TRIGGER */
     EXPR_KIND_POLICY,            /* USING or WITH CHECK expr in policy */
     EXPR_KIND_PARTITION_EXPRESSION, /* PARTITION BY expression */
+    EXPR_KIND_PARTITION_BOUNDS,     /* partition bounds value */
     EXPR_KIND_CALL_ARGUMENT        /* procedure argument in CALL */
 } ParseExprKind;
 
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index e724439037..c1ead46a7d 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -308,7 +308,7 @@ CREATE TABLE partitioned (
     a int,
     b int
 ) PARTITION BY RANGE ((avg(a) OVER (PARTITION BY b)));
-ERROR:  window functions are not allowed in partition key expressions
+ERROR:  window functions are not allowed in partition bounds
 CREATE TABLE partitioned (
     a int
 ) PARTITION BY LIST ((a LIKE (SELECT 1)));
@@ -450,13 +450,9 @@ CREATE TABLE part_1 PARTITION OF list_parted FOR VALUES IN ('1');
 CREATE TABLE part_2 PARTITION OF list_parted FOR VALUES IN (2);
 CREATE TABLE part_null PARTITION OF list_parted FOR VALUES IN (null);
 CREATE TABLE fail_part PARTITION OF list_parted FOR VALUES IN (int '1');
-ERROR:  syntax error at or near "int"
-LINE 1: ... fail_part PARTITION OF list_parted FOR VALUES IN (int '1');
-                                                              ^
+ERROR:  partition "fail_part" would overlap partition "part_1"
 CREATE TABLE fail_part PARTITION OF list_parted FOR VALUES IN ('1'::int);
-ERROR:  syntax error at or near "::"
-LINE 1: ...fail_part PARTITION OF list_parted FOR VALUES IN ('1'::int);
-                                                                ^
+ERROR:  partition "fail_part" would overlap partition "part_1"
 -- syntax does not allow empty list of values for list partitions
 CREATE TABLE fail_part PARTITION OF list_parted FOR VALUES IN ();
 ERROR:  syntax error at or near ")"
@@ -490,12 +486,10 @@ 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_11 PARTITION OF moneyp FOR VALUES IN ('11');
+CREATE TABLE moneyp_12 PARTITION OF moneyp FOR VALUES IN (to_char(12, '99')::int);
 CREATE TABLE moneyp_10 PARTITION OF moneyp FOR VALUES IN ('10');
+ERROR:  relation "moneyp_10" already exists
 DROP TABLE moneyp;
 -- immutable cast should work, though
 CREATE TABLE bigintp (
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 235bef13dc..a626bfe659 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -458,6 +458,8 @@ CREATE TABLE moneyp (
     a money
 ) PARTITION BY LIST (a);
 CREATE TABLE moneyp_10 PARTITION OF moneyp FOR VALUES IN (10);
+CREATE TABLE moneyp_11 PARTITION OF moneyp FOR VALUES IN ('11');
+CREATE TABLE moneyp_12 PARTITION OF moneyp FOR VALUES IN (to_char(12, '99')::int);
 CREATE TABLE moneyp_10 PARTITION OF moneyp FOR VALUES IN ('10');
 DROP TABLE moneyp;