Re: using expression syntax for partition bounds - Mailing list pgsql-hackers
From | Kyotaro HORIGUCHI |
---|---|
Subject | Re: using expression syntax for partition bounds |
Date | |
Msg-id | 20180424.180838.65007339.horiguchi.kyotaro@lab.ntt.co.jp Whole thread Raw |
In response to | Re: using expression syntax for partition bounds (was: Re: Booleanpartitions syntax) (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>) |
Responses |
Re: using expression syntax for partition bounds
|
List | pgsql-hackers |
Thanks. I have almost missed this. At Mon, 23 Apr 2018 11:44:14 +0900, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote in <e8c770eb-a850-6645-8310-f3eaea3a72fd@lab.ntt.co.jp> > 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 <replaceable class="parameter">numeric_literal</replaceable>, REM <para> Each of the values specified in the <replaceable class="parameter">partition_bound_spec</replaceable> is - a literal, <literal>NULL</literal>, <literal>MINVALUE</literal>, or - <literal>MAXVALUE</literal>. 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. </para> <para> 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 result_collation) { diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 5a36367446..8b0680a7fa 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -581,8 +581,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 <list> hash_partbound partbound_datum_list range_datum_list +%type <list> hash_partbound %type <defelt> hash_partbound_elem /* @@ -2739,7 +2738,7 @@ PartitionBoundSpec: } /* a LIST partition */ - | FOR VALUES IN_P '(' partbound_datum_list ')' + | FOR VALUES IN_P '(' expr_list ')' { PartitionBoundSpec *n = makeNode(PartitionBoundSpec); @@ -2752,7 +2751,7 @@ PartitionBoundSpec: } /* a RANGE partition */ - | FOR VALUES FROM '(' range_datum_list ')' TO '(' range_datum_list ')' + | FOR VALUES FROM '(' expr_list ')' TO '(' expr_list ')' { PartitionBoundSpec *n = makeNode(PartitionBoundSpec); @@ -2795,59 +2794,6 @@ hash_partbound: } ; -partbound_datum: - Sconst { $$ = makeStringConst($1, @1); } - | NumericOnly { $$ = makeAConst($1, @1); } - | TRUE_P { $$ = makeStringConst(pstrdup("true"), @1); } - | FALSE_P { $$ = makeStringConst(pstrdup("false"), @1); } - | NULL_P { $$ = makeNullAConst(@1); } - ; - -partbound_datum_list: - partbound_datum { $$ = list_make1($1); } - | partbound_datum_list ',' partbound_datum - { $$ = lappend($1, $3); } - ; - -range_datum_list: - PartitionRangeDatum { $$ = list_make1($1); } - | range_datum_list ',' PartitionRangeDatum - { $$ = lappend($1, $3); } - ; - -PartitionRangeDatum: - MINVALUE - { - PartitionRangeDatum *n = makeNode(PartitionRangeDatum); - - n->kind = PARTITION_RANGE_DATUM_MINVALUE; - n->value = NULL; - n->location = @1; - - $$ = (Node *) n; - } - | MAXVALUE - { - PartitionRangeDatum *n = makeNode(PartitionRangeDatum); - - n->kind = PARTITION_RANGE_DATUM_MAXVALUE; - n->value = NULL; - n->location = @1; - - $$ = (Node *) n; - } - | partbound_datum - { - PartitionRangeDatum *n = makeNode(PartitionRangeDatum); - - n->kind = PARTITION_RANGE_DATUM_VALUE; - n->value = $1; - n->location = @1; - - $$ = (Node *) n; - } - ; - /***************************************************************************** * * ALTER TYPE diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c index 61727e1d71..55a5304a38 100644 --- a/src/backend/parser/parse_agg.c +++ b/src/backend/parser/parse_agg.c @@ -499,6 +499,13 @@ check_agglevels_and_constraints(ParseState *pstate, Node *expr) else err = _("grouping operations are not allowed in EXECUTE parameters"); + break; + case EXPR_KIND_PARTITION_BOUND: + if (isAgg) + err = _("aggregate functions are not allowed in partition bound"); + else + err = _("grouping operations are not allowed in partition bound"); + break; case EXPR_KIND_TRIGGER_WHEN: if (isAgg) @@ -899,6 +906,9 @@ transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc, case EXPR_KIND_PARTITION_EXPRESSION: err = _("window functions are not allowed in partition key expressions"); break; + case EXPR_KIND_PARTITION_BOUND: + err = _("window functions are not allowed in partition bound"); + break; case EXPR_KIND_CALL_ARGUMENT: err = _("window functions are not allowed in CALL arguments"); break; diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 385e54a9b6..f8759f185b 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -1849,6 +1849,9 @@ transformSubLink(ParseState *pstate, SubLink *sublink) case EXPR_KIND_CALL_ARGUMENT: err = _("cannot use subquery in CALL argument"); break; + case EXPR_KIND_PARTITION_BOUND: + err = _("cannot use subquery in partition bound"); + break; /* * There is intentionally no default: case here, so that the @@ -3473,6 +3476,8 @@ ParseExprKindName(ParseExprKind exprKind) return "WHEN"; case EXPR_KIND_PARTITION_EXPRESSION: return "PARTITION BY"; + case EXPR_KIND_PARTITION_BOUND: + return "partition bound"; case EXPR_KIND_CALL_ARGUMENT: return "CALL"; diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index ea5d5212b4..570ae860ae 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -2303,6 +2303,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_BOUND: + err = _("set-returning functions are not allowed in partition bound"); + 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 c6f3628def..a948225bec 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,9 +139,12 @@ static void transformConstraintAttrs(CreateStmtContext *cxt, 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 transformPartitionRangeBounds(ParseState *pstate, List *blist); static void validateInfiniteBounds(ParseState *pstate, List *blist); -static Const *transformPartitionBoundValue(ParseState *pstate, A_Const *con, - const char *colName, Oid colType, int32 colTypmod); +static Node *boundValuePreColumnRefCheck(ParseState *pstate, ColumnRef *cref); +static Const *transformPartitionBoundValue(ParseState *pstate, Node *con, + const char *colName, Oid colType, int32 colTypmod, + Oid partCollation); /* @@ -3649,6 +3653,7 @@ transformPartitionBound(ParseState *pstate, Relation parent, char *colname; Oid coltype; int32 coltypmod; + Oid partcollation; if (spec->strategy != PARTITION_STRATEGY_LIST) ereport(ERROR, @@ -3668,17 +3673,19 @@ transformPartitionBound(ParseState *pstate, Relation parent, /* Need its type data too */ coltype = get_partition_col_typid(key, 0); coltypmod = get_partition_col_typmod(key, 0); + partcollation = get_partition_col_collation(key, 0); 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, - colname, coltype, coltypmod); + value = transformPartitionBoundValue(pstate, expr, + colname, coltype, coltypmod, + partcollation); /* Don't add to the result if the value is a duplicate */ duplicate = false; @@ -3725,7 +3732,9 @@ transformPartitionBound(ParseState *pstate, Relation parent, * Once we see MINVALUE or MAXVALUE for one column, the remaining * columns must be the same. */ + transformPartitionRangeBounds(pstate, spec->lowerdatums); validateInfiniteBounds(pstate, spec->lowerdatums); + transformPartitionRangeBounds(pstate, spec->upperdatums); validateInfiniteBounds(pstate, spec->upperdatums); /* Transform all the constants */ @@ -3738,8 +3747,8 @@ transformPartitionBound(ParseState *pstate, Relation parent, char *colname; Oid coltype; int32 coltypmod; - A_Const *con; Const *value; + Oid partcollation; /* Get the column's name in case we need to output an error */ if (key->partattrs[i] != 0) @@ -3756,13 +3765,15 @@ transformPartitionBound(ParseState *pstate, Relation parent, /* Need its type data too */ coltype = get_partition_col_typid(key, i); coltypmod = get_partition_col_typmod(key, i); + partcollation = get_partition_col_collation(key, i); if (ldatum->value) { - con = castNode(A_Const, ldatum->value); - value = transformPartitionBoundValue(pstate, con, + value = transformPartitionBoundValue(pstate, + ldatum->value, colname, - coltype, coltypmod); + coltype, coltypmod, + partcollation); if (value->constisnull) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), @@ -3773,10 +3784,11 @@ 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); + coltype, coltypmod, + partcollation); if (value->constisnull) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), @@ -3799,6 +3811,65 @@ transformPartitionBound(ParseState *pstate, Relation parent, return result_spec; } +/* + * transformPartitionRangeBounds + * This converts the expressions for range partition bounds from the raw + * grammar representation to PartitionRangeDatum structs + */ +static void +transformPartitionRangeBounds(ParseState *pstate, List *blist) +{ + ListCell *lc; + + foreach(lc, blist) + { + Node *expr = lfirst(lc); + PartitionRangeDatum *prd = makeNode(PartitionRangeDatum); + + /* + * Infinite range bounds -- "minvalue" and "maxvalue" -- get passed + * in as ColumnRefs. + */ + if (IsA(expr, ColumnRef)) + { + ColumnRef *cref = (ColumnRef *) expr; + char *cname; + + if (list_length(cref->fields) == 1 && + IsA(linitial(cref->fields), String)) + cname = strVal(linitial(cref->fields)); + + if (strcmp("minvalue", cname) == 0) + { + prd->kind = PARTITION_RANGE_DATUM_MINVALUE; + prd->value = NULL; + prd->location = exprLocation(expr); + } + else if (strcmp("maxvalue", cname) == 0) + { + prd->kind = PARTITION_RANGE_DATUM_MAXVALUE; + prd->value = NULL; + prd->location = exprLocation(expr); + } + else + { + /* to issue ERROR for column reference */ + boundValuePreColumnRefCheck(pstate, (ColumnRef *) expr); + /* not reaches here */ + } + } + else + { + prd->kind = PARTITION_RANGE_DATUM_VALUE; + prd->value = expr; + prd->location = exprLocation(expr); + } + + /* Use the existing List structure. */ + lfirst(lc) = prd; + } +} + /* * validateInfiniteBounds * @@ -3839,17 +3910,38 @@ validateInfiniteBounds(ParseState *pstate, List *blist) } } +/* + * Column references are not allowed in partition bound values. Set this + * callback to error out them in transformExpr. + */ +static Node * +boundValuePreColumnRefCheck(ParseState *pstate, ColumnRef *cref) +{ + ereport(ERROR, + (errcode (ERRCODE_SYNTAX_ERROR), + errmsg ("column reference is not allowed in bound value"), + parser_errposition(pstate, exprLocation((Node *) cref)))); +} + /* * Transform one constant in a partition bound spec */ static Const * -transformPartitionBoundValue(ParseState *pstate, A_Const *con, - const char *colName, Oid colType, int32 colTypmod) +transformPartitionBoundValue(ParseState *pstate, Node *val, + const char *colName, Oid colType, int32 colTypmod, + Oid partCollation) { Node *value; + PreParseColumnRefHook oldhook = pstate->p_pre_columnref_hook; - /* Make it into a Const */ - value = (Node *) make_const(pstate, &con->val, con->location); + /* We reject column references within val */ + pstate->p_pre_columnref_hook = boundValuePreColumnRefCheck; + + /* Transform raw parsetree */ + value = transformExpr(pstate, val, EXPR_KIND_PARTITION_BOUND); + + /* Do we need to restore the hook? */ + pstate->p_pre_columnref_hook = oldhook; /* Coerce to correct type */ value = coerce_to_target_type(pstate, @@ -3865,21 +3957,20 @@ 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 */ + /* We ignore the value's collation */ if (!IsA(value, Const)) - value = (Node *) expression_planner((Expr *) value); + value = (Node *) eval_const_expressions(NULL, value); - /* Fail if we don't have a constant (i.e., non-immutable coercion) */ + /* + * If it is not a Const yet, Evaluate the expression applying given + * collation. + */ 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))); + value = (Node *) evaluate_expr((Expr *) value, colType, colTypmod, + partCollation); + Assert(IsA(value, Const)); return (Const *) value; } diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h index ed854fdd40..06034d499b 100644 --- a/src/include/optimizer/clauses.h +++ b/src/include/optimizer/clauses.h @@ -88,4 +88,6 @@ extern Query *inline_set_returning_function(PlannerInfo *root, extern List *expand_function_arguments(List *args, Oid result_type, HeapTuple func_tuple); +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 0230543810..68bec4b932 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -69,6 +69,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_BOUND, /* partition bounds value */ EXPR_KIND_CALL_ARGUMENT /* procedure argument in CALL */ } ParseExprKind; diff --git a/src/include/utils/partcache.h b/src/include/utils/partcache.h index c1d029fdb3..105e76ced3 100644 --- a/src/include/utils/partcache.h +++ b/src/include/utils/partcache.h @@ -93,4 +93,10 @@ get_partition_col_typmod(PartitionKey key, int col) return key->parttypmod[col]; } +static inline Oid +get_partition_col_collation(PartitionKey key, int col) +{ + return key->partcollation[col]; +} + #endif /* PARTCACHE_H */ diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 470fca0cab..264cbcbefe 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -449,14 +449,6 @@ CREATE TABLE list_parted ( 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'); - ^ -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); - ^ -- 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 +482,8 @@ 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'); +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); 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 140bf41f76..5ba35038f5 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -432,8 +432,6 @@ CREATE TABLE list_parted ( 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'); -CREATE TABLE fail_part PARTITION OF list_parted FOR VALUES IN ('1'::int); -- syntax does not allow empty list of values for list partitions CREATE TABLE fail_part PARTITION OF list_parted FOR VALUES IN (); @@ -458,7 +456,8 @@ CREATE TABLE moneyp ( a money ) PARTITION BY LIST (a); CREATE TABLE moneyp_10 PARTITION OF moneyp FOR VALUES IN (10); -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); DROP TABLE moneyp; -- immutable cast should work, though 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 <replaceable class="parameter">numeric_literal</replaceable>, REM <para> Each of the values specified in the <replaceable class="parameter">partition_bound_spec</replaceable> is - a literal, <literal>NULL</literal>, <literal>MINVALUE</literal>, or - <literal>MAXVALUE</literal>. 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. </para> <para> 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 result_collation) { diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 5a36367446..8b0680a7fa 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -581,8 +581,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 <list> hash_partbound partbound_datum_list range_datum_list +%type <list> hash_partbound %type <defelt> hash_partbound_elem /* @@ -2739,7 +2738,7 @@ PartitionBoundSpec: } /* a LIST partition */ - | FOR VALUES IN_P '(' partbound_datum_list ')' + | FOR VALUES IN_P '(' expr_list ')' { PartitionBoundSpec *n = makeNode(PartitionBoundSpec); @@ -2752,7 +2751,7 @@ PartitionBoundSpec: } /* a RANGE partition */ - | FOR VALUES FROM '(' range_datum_list ')' TO '(' range_datum_list ')' + | FOR VALUES FROM '(' expr_list ')' TO '(' expr_list ')' { PartitionBoundSpec *n = makeNode(PartitionBoundSpec); @@ -2795,59 +2794,6 @@ hash_partbound: } ; -partbound_datum: - Sconst { $$ = makeStringConst($1, @1); } - | NumericOnly { $$ = makeAConst($1, @1); } - | TRUE_P { $$ = makeStringConst(pstrdup("true"), @1); } - | FALSE_P { $$ = makeStringConst(pstrdup("false"), @1); } - | NULL_P { $$ = makeNullAConst(@1); } - ; - -partbound_datum_list: - partbound_datum { $$ = list_make1($1); } - | partbound_datum_list ',' partbound_datum - { $$ = lappend($1, $3); } - ; - -range_datum_list: - PartitionRangeDatum { $$ = list_make1($1); } - | range_datum_list ',' PartitionRangeDatum - { $$ = lappend($1, $3); } - ; - -PartitionRangeDatum: - MINVALUE - { - PartitionRangeDatum *n = makeNode(PartitionRangeDatum); - - n->kind = PARTITION_RANGE_DATUM_MINVALUE; - n->value = NULL; - n->location = @1; - - $$ = (Node *) n; - } - | MAXVALUE - { - PartitionRangeDatum *n = makeNode(PartitionRangeDatum); - - n->kind = PARTITION_RANGE_DATUM_MAXVALUE; - n->value = NULL; - n->location = @1; - - $$ = (Node *) n; - } - | partbound_datum - { - PartitionRangeDatum *n = makeNode(PartitionRangeDatum); - - n->kind = PARTITION_RANGE_DATUM_VALUE; - n->value = $1; - n->location = @1; - - $$ = (Node *) n; - } - ; - /***************************************************************************** * * ALTER TYPE diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c index 61727e1d71..55a5304a38 100644 --- a/src/backend/parser/parse_agg.c +++ b/src/backend/parser/parse_agg.c @@ -499,6 +499,13 @@ check_agglevels_and_constraints(ParseState *pstate, Node *expr) else err = _("grouping operations are not allowed in EXECUTE parameters"); + break; + case EXPR_KIND_PARTITION_BOUND: + if (isAgg) + err = _("aggregate functions are not allowed in partition bound"); + else + err = _("grouping operations are not allowed in partition bound"); + break; case EXPR_KIND_TRIGGER_WHEN: if (isAgg) @@ -899,6 +906,9 @@ transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc, case EXPR_KIND_PARTITION_EXPRESSION: err = _("window functions are not allowed in partition key expressions"); break; + case EXPR_KIND_PARTITION_BOUND: + err = _("window functions are not allowed in partition bound"); + break; case EXPR_KIND_CALL_ARGUMENT: err = _("window functions are not allowed in CALL arguments"); break; diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 385e54a9b6..f8759f185b 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -1849,6 +1849,9 @@ transformSubLink(ParseState *pstate, SubLink *sublink) case EXPR_KIND_CALL_ARGUMENT: err = _("cannot use subquery in CALL argument"); break; + case EXPR_KIND_PARTITION_BOUND: + err = _("cannot use subquery in partition bound"); + break; /* * There is intentionally no default: case here, so that the @@ -3473,6 +3476,8 @@ ParseExprKindName(ParseExprKind exprKind) return "WHEN"; case EXPR_KIND_PARTITION_EXPRESSION: return "PARTITION BY"; + case EXPR_KIND_PARTITION_BOUND: + return "partition bound"; case EXPR_KIND_CALL_ARGUMENT: return "CALL"; diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index ea5d5212b4..570ae860ae 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -2303,6 +2303,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_BOUND: + err = _("set-returning functions are not allowed in partition bound"); + 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 c6f3628def..ba9170ce7e 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -48,7 +48,9 @@ #include "miscadmin.h" #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" +#include "optimizer/clauses.h" #include "optimizer/planner.h" +#include "optimizer/var.h" #include "parser/analyze.h" #include "parser/parse_clause.h" #include "parser/parse_coerce.h" @@ -138,9 +140,13 @@ static void transformConstraintAttrs(CreateStmtContext *cxt, 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 transformPartitionRangeBounds(ParseState *pstate, List *blist, + const char *colName, Oid colType, int32 colTypmod, + Oid partCollation); static void validateInfiniteBounds(ParseState *pstate, List *blist); -static Const *transformPartitionBoundValue(ParseState *pstate, A_Const *con, - const char *colName, Oid colType, int32 colTypmod); +static Const *transformPartitionBoundValue(ParseState *pstate, Node *con, + const char *colName, Oid colType, int32 colTypmod, + Oid partCollation); /* @@ -3599,6 +3605,7 @@ transformPartitionBound(ParseState *pstate, Relation parent, { PartitionBoundSpec *result_spec; PartitionKey key = RelationGetPartitionKey(parent); + RangeTblEntry *rte; char strategy = get_partition_strategy(key); int partnatts = get_partition_natts(key); List *partexprs = get_partition_exprs(key); @@ -3623,6 +3630,13 @@ transformPartitionBound(ParseState *pstate, Relation parent, return result_spec; } + /* + * Add RangeTblEntry for the relation. We assume this function is not + * called recursively. + */ + rte = addRangeTableEntryForRelation(pstate, parent, NULL, false, true); + addRTEtoQuery(pstate, rte, true, true, true); + if (strategy == PARTITION_STRATEGY_HASH) { if (spec->strategy != PARTITION_STRATEGY_HASH) @@ -3649,6 +3663,7 @@ transformPartitionBound(ParseState *pstate, Relation parent, char *colname; Oid coltype; int32 coltypmod; + Oid partcollation; if (spec->strategy != PARTITION_STRATEGY_LIST) ereport(ERROR, @@ -3668,17 +3683,19 @@ transformPartitionBound(ParseState *pstate, Relation parent, /* Need its type data too */ coltype = get_partition_col_typid(key, 0); coltypmod = get_partition_col_typmod(key, 0); + partcollation = get_partition_col_collation(key, 0); 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, - colname, coltype, coltypmod); + value = transformPartitionBoundValue(pstate, expr, + colname, coltype, coltypmod, + partcollation); /* Don't add to the result if the value is a duplicate */ duplicate = false; @@ -3701,6 +3718,10 @@ transformPartitionBound(ParseState *pstate, Relation parent, } else if (strategy == PARTITION_STRATEGY_RANGE) { + char *colname; + Oid coltype; + int32 coltypmod; + Oid partcollation; ListCell *cell1, *cell2; int i, @@ -3721,11 +3742,31 @@ transformPartitionBound(ParseState *pstate, Relation parent, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("TO must specify exactly one value per partitioning column"))); + /* Get the only column's name in case we need to output an error */ + if (key->partattrs[0] != 0) + colname = get_attname(RelationGetRelid(parent), + key->partattrs[0], false); + else + colname = deparse_expression((Node *) linitial(partexprs), + deparse_context_for(RelationGetRelationName(parent), + RelationGetRelid(parent)), + false, false); + /* Need its type data too */ + coltype = get_partition_col_typid(key, 0); + coltypmod = get_partition_col_typmod(key, 0); + partcollation = get_partition_col_collation(key, 0); + /* * Once we see MINVALUE or MAXVALUE for one column, the remaining * columns must be the same. */ + transformPartitionRangeBounds(pstate, spec->lowerdatums, + colname, coltype, coltypmod, + partcollation); validateInfiniteBounds(pstate, spec->lowerdatums); + transformPartitionRangeBounds(pstate, spec->upperdatums, + colname, coltype, coltypmod, + partcollation); validateInfiniteBounds(pstate, spec->upperdatums); /* Transform all the constants */ @@ -3738,8 +3779,8 @@ transformPartitionBound(ParseState *pstate, Relation parent, char *colname; Oid coltype; int32 coltypmod; - A_Const *con; Const *value; + Oid partcollation; /* Get the column's name in case we need to output an error */ if (key->partattrs[i] != 0) @@ -3756,13 +3797,15 @@ transformPartitionBound(ParseState *pstate, Relation parent, /* Need its type data too */ coltype = get_partition_col_typid(key, i); coltypmod = get_partition_col_typmod(key, i); + partcollation = get_partition_col_collation(key, i); if (ldatum->value) { - con = castNode(A_Const, ldatum->value); - value = transformPartitionBoundValue(pstate, con, + value = transformPartitionBoundValue(pstate, + ldatum->value, colname, - coltype, coltypmod); + coltype, coltypmod, + partcollation); if (value->constisnull) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), @@ -3773,10 +3816,11 @@ 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); + coltype, coltypmod, + partcollation); if (value->constisnull) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), @@ -3799,6 +3843,70 @@ transformPartitionBound(ParseState *pstate, Relation parent, return result_spec; } +/* + * transformPartitionRangeBounds + * This converts the expressions for range partition bounds from the raw + * grammar representation to PartitionRangeDatum structs + */ +static void +transformPartitionRangeBounds(ParseState *pstate, List *blist, + const char *colName, Oid colType, int32 colTypmod, + Oid partCollation) +{ + ListCell *lc; + + foreach(lc, blist) + { + Node *expr = lfirst(lc); + PartitionRangeDatum *prd = makeNode(PartitionRangeDatum); + bool done = false; + + /* + * Infinite range bounds -- "minvalue" and "maxvalue" -- get passed + * in as ColumnRefs. + */ + if (IsA(expr, ColumnRef)) + { + ColumnRef *cref = (ColumnRef *) expr; + char *cname; + + if (list_length(cref->fields) == 1 && + IsA(linitial(cref->fields), String)) + cname = strVal(linitial(cref->fields)); + + if (strcmp("minvalue", cname) == 0) + { + prd->kind = PARTITION_RANGE_DATUM_MINVALUE; + prd->value = NULL; + prd->location = exprLocation(expr); + done = true; + } + else if (strcmp("maxvalue", cname) == 0) + { + prd->kind = PARTITION_RANGE_DATUM_MAXVALUE; + prd->value = NULL; + prd->location = exprLocation(expr); + done = true; + } + } + + if (!done) + { + Const *value = transformPartitionBoundValue(pstate, + (Node *) expr, + colName, colType, + colTypmod, + partCollation); + prd->kind = PARTITION_RANGE_DATUM_VALUE; + prd->value = (Node *) value; + prd->location = exprLocation(expr); + } + + /* Use the existing List structure. */ + lfirst(lc) = prd; + } +} + /* * validateInfiniteBounds * @@ -3843,13 +3951,22 @@ validateInfiniteBounds(ParseState *pstate, List *blist) * Transform one constant in a partition bound spec */ static Const * -transformPartitionBoundValue(ParseState *pstate, A_Const *con, - const char *colName, Oid colType, int32 colTypmod) +transformPartitionBoundValue(ParseState *pstate, Node *val, + const char *colName, Oid colType, int32 colTypmod, + Oid partCollation) { Node *value; - /* Make it into a Const */ - value = (Node *) make_const(pstate, &con->val, con->location); + /* Transform raw parsetree */ + value = transformExpr(pstate, val, EXPR_KIND_PARTITION_BOUND); + + /* No column reference is allowed */ + if (contain_var_clause(value)) + ereport(ERROR, + (errcode (ERRCODE_SYNTAX_ERROR), + errmsg ("cannot use column referencees in bound value"), + parser_errposition(pstate, exprLocation((Node *) val)))); + /* Coerce to correct type */ value = coerce_to_target_type(pstate, @@ -3865,21 +3982,20 @@ 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 */ + /* We ignore the value's collation */ if (!IsA(value, Const)) - value = (Node *) expression_planner((Expr *) value); + value = (Node *) eval_const_expressions(NULL, value); - /* Fail if we don't have a constant (i.e., non-immutable coercion) */ + /* + * If it is not a Const yet, Evaluate the expression applying given + * collation. + */ 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))); + value = (Node *) evaluate_expr((Expr *) value, colType, colTypmod, + partCollation); + Assert(IsA(value, Const)); return (Const *) value; } diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h index ed854fdd40..06034d499b 100644 --- a/src/include/optimizer/clauses.h +++ b/src/include/optimizer/clauses.h @@ -88,4 +88,6 @@ extern Query *inline_set_returning_function(PlannerInfo *root, extern List *expand_function_arguments(List *args, Oid result_type, HeapTuple func_tuple); +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 0230543810..68bec4b932 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -69,6 +69,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_BOUND, /* partition bounds value */ EXPR_KIND_CALL_ARGUMENT /* procedure argument in CALL */ } ParseExprKind; diff --git a/src/include/utils/partcache.h b/src/include/utils/partcache.h index c1d029fdb3..105e76ced3 100644 --- a/src/include/utils/partcache.h +++ b/src/include/utils/partcache.h @@ -93,4 +93,10 @@ get_partition_col_typmod(PartitionKey key, int col) return key->parttypmod[col]; } +static inline Oid +get_partition_col_collation(PartitionKey key, int col) +{ + return key->partcollation[col]; +} + #endif /* PARTCACHE_H */ diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 470fca0cab..264cbcbefe 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -449,14 +449,6 @@ CREATE TABLE list_parted ( 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'); - ^ -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); - ^ -- 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 +482,8 @@ 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'); +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); 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 140bf41f76..5ba35038f5 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -432,8 +432,6 @@ CREATE TABLE list_parted ( 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'); -CREATE TABLE fail_part PARTITION OF list_parted FOR VALUES IN ('1'::int); -- syntax does not allow empty list of values for list partitions CREATE TABLE fail_part PARTITION OF list_parted FOR VALUES IN (); @@ -458,7 +456,8 @@ CREATE TABLE moneyp ( a money ) PARTITION BY LIST (a); CREATE TABLE moneyp_10 PARTITION OF moneyp FOR VALUES IN (10); -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); DROP TABLE moneyp; -- immutable cast should work, though
pgsql-hackers by date: