From 33b64b30394880dd2701359a4903dce377f41d00 Mon Sep 17 00:00:00 2001 From: amit Date: Wed, 18 Apr 2018 18:53:44 +0900 Subject: [PATCH v7] Allow generalized expression syntax for partition bounds This fixes the bug that Boolean literals true/false are allowed in partition bound syntax. --- src/backend/optimizer/util/clauses.c | 4 +- src/backend/parser/gram.y | 32 +++++++++++---- 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 | 66 +++++++++++++++++------------- 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 +-- 11 files changed, 92 insertions(+), 58 deletions(-) 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 e548476623..e0b54dc06f 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -468,8 +468,9 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type columnDef columnOptions %type def_elem reloption_elem old_aggr_elem operator_def_elem %type def_arg columnElem where_clause where_or_current_clause - a_expr b_expr c_expr AexprConst indirection_el opt_slice_bound - columnref in_expr having_clause func_table xmltable array_expr + a_expr a_expr_no_columnref b_expr c_expr c_expr_no_columnref + AexprConst indirection_el opt_slice_bound columnref in_expr + having_clause func_table xmltable array_expr ExclusionWhereClause operator_def_arg %type rowsfrom_item rowsfrom_list opt_col_def_list %type opt_ordinality @@ -2796,9 +2797,7 @@ hash_partbound: ; partbound_datum: - Sconst { $$ = makeStringConst($1, @1); } - | NumericOnly { $$ = makeAConst($1, @1); } - | NULL_P { $$ = makeNullAConst(@1); } + a_expr_no_columnref { $$ = $1; } ; partbound_datum_list: @@ -12897,6 +12896,10 @@ interval_second: * because that use of AND conflicts with AND as a boolean operator. So, * b_expr is used in BETWEEN and we remove boolean keywords from b_expr. * + * a_expr_no_columnref is separately defined to be used in productions for + * partition bound expressions, which also match unreserved keywords + * MAXVALUE and MINVALUE. + * * Note that '(' a_expr ')' is a b_expr, so an unrestricted expression can * always be used by surrounding it with parens. * @@ -12909,7 +12912,12 @@ interval_second: * of the first terminal instead; otherwise you will not get the behavior * you expect! So we use %prec annotations freely to set precedences. */ -a_expr: c_expr { $$ = $1; } +a_expr: a_expr_no_columnref { $$ = $1; } + | columnref { $$ = $1; } + ; + +a_expr_no_columnref: + c_expr_no_columnref { $$ = $1; } | a_expr TYPECAST Typename { $$ = makeTypeCast($1, $3, @2); } | a_expr COLLATE any_name @@ -13331,7 +13339,9 @@ 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 +b_expr: c_expr_no_columnref + { $$ = $1; } + | columnref { $$ = $1; } | b_expr TYPECAST Typename { $$ = makeTypeCast($1, $3, @2); } @@ -13406,8 +13416,12 @@ b_expr: c_expr * inside parentheses, such as function arguments; that cannot introduce * ambiguity to the b_expr syntax. */ -c_expr: columnref { $$ = $1; } - | AexprConst { $$ = $1; } +c_expr: c_expr_no_columnref { $$ = $1; } + | columnref { $$ = $1; } + ; + +c_expr_no_columnref: + AexprConst { $$ = $1; } | PARAM opt_indirection { ParamRef *p = makeNode(ParamRef); 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 @@ -500,6 +500,13 @@ check_agglevels_and_constraints(ParseState *pstate, Node *expr) 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) err = _("aggregate functions are not allowed in trigger WHEN conditions"); @@ -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..1452c32ab2 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" @@ -139,8 +140,9 @@ 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, - const char *colName, Oid colType, int32 colTypmod); +static Const *transformPartitionBoundValue(ParseState *pstate, Node *con, + const char *colName, Oid colType, int32 colTypmod, + Oid partCollation); /* @@ -3649,6 +3651,7 @@ transformPartitionBound(ParseState *pstate, Relation parent, char *colname; Oid coltype; int32 coltypmod; + Oid partcollation; if (spec->strategy != PARTITION_STRATEGY_LIST) ereport(ERROR, @@ -3668,17 +3671,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; @@ -3738,8 +3743,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 +3761,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 +3780,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), @@ -3843,13 +3851,14 @@ 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); /* Coerce to correct type */ value = coerce_to_target_type(pstate, @@ -3865,21 +3874,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 */ + /* + * 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; + + /* Evaluate the expression. */ if (!IsA(value, Const)) - value = (Node *) expression_planner((Expr *) value); - - /* Fail if we 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))); + 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 654464c631..0ebe8dc50e 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 54694347ae..8711ead7cc 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 -- 2.11.0