Re: Boolean partitions syntax - Mailing list pgsql-hackers

Hello, I returned to this.

I'd like to insisnt on prposing to use existing parser element.

At Mon, 9 Apr 2018 10:11:08 -0400, "Jonathan S. Katz" <jonathan.katz@excoventures.com> wrote in
<27021281-2ED7-4CDE-9D82-366AF10B3B57@excoventures.com>
> > On Apr 9, 2018, at 10:06 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > It's premature to discuss whether this could be back-patched when
> > we haven't got an acceptable patch yet.
> 
> Understood.

At Fri, 2 Mar 2018 16:49:29 +0900, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote in
<2d49cd86-acb9-fc5b-8eb7-e467b01ec25a@lab.ntt.co.jp>
> I had tried the approach your patch takes and had noticed that the syntax
> had stopped accepting negative values (a regression test fails due to that).
...
> I had tried fixing that as well, but it didn't readily work.

Just adding negation would work as a_expr is doing.

>             | '-' a_expr                    %prec UMINUS
>                 { $$ = doNegate($2, @1); }

Boolean and negative values are accepted as partition bound
values with the attached patch.

> =# create table p (a bool, b int) partition by list(a);
> CREATE TABLE
> =# create table ct partition of p for values in (true) partition by list (b);
> CREATE TABLE
> =# create table ct1 partition of ct for values in (-1, -2);
> CREATE TABLE

And illegal negative is rejected.

> =# create table ct3 partition of ct for values in (-true);
> ERROR:  operator does not exist: - boolean
> LINE 1: create table ct3 partition of ct for values in (-true);

Interval has a conversion to text so this behaves someshat oddly
but it seems to me that we don't need reject that explicitly.

> create table c2 partition of p for values in (interval '1 year');
> CREATE TABLE
> =# \d+ c2
...
> Partition of: p FOR VALUES IN ('1 year')

or

> =# create table c1 partition of p for values in (interval '1 day');
> ERROR:  specified value cannot be cast to type integer for column "a"
> LINE 1: create table c1 partition of p for values in (interval '1 da...

The attached patch is adding lines for error checking in some
functions like transformWindowFuncCall. They are basically
useless as they are to be rejected by parser but it seems to be
needed for consistency.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index dd0c26c11b..710f7a9dc1 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2805,9 +2805,9 @@ hash_partbound:
         ;
 
 partbound_datum:
-            Sconst            { $$ = makeStringConst($1, @1); }
-            | NumericOnly    { $$ = makeAConst($1, @1); }
-            | NULL_P        { $$ = makeNullAConst(@1); }
+        AexprConst            { $$ = $1; }
+        | '-' AexprConst            %prec UMINUS
+                            { $$ = doNegate($2, @1); }
         ;
 
 partbound_datum_list:
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..c14f1265a9 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -138,7 +138,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 +3674,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 +3740,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 +3760,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 +3774,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 +3844,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,7 +3866,7 @@ 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))
@@ -3881,7 +3880,7 @@ transformPartitionBoundValue(ParseState *pstate, A_Const *con,
                         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)));
+                 parser_errposition(pstate, exprLocation(val))));
 
     return (Const *) value;
 }
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;