From e82738affc8f3e4c52b1ee6dd6a54cf23b991d6c Mon Sep 17 00:00:00 2001 From: amit Date: Wed, 18 Apr 2018 18:53:44 +0900 Subject: [PATCH v1] Allow generalized expression syntax for partition bounds This fixes the bug that Boolean literals true/false are not allowed in partition bound syntax. Authors: Kyotaro Horiguchi, Amit Langote --- doc/src/sgml/ref/alter_table.sgml | 6 +- doc/src/sgml/ref/create_table.sgml | 14 ++-- src/backend/optimizer/util/clauses.c | 4 +- src/backend/parser/gram.y | 58 +------------ src/backend/parser/parse_agg.c | 10 +++ src/backend/parser/parse_expr.c | 5 ++ src/backend/parser/parse_func.c | 3 + src/backend/parser/parse_utilcmd.c | 126 ++++++++++++++++++++++------- src/include/optimizer/clauses.h | 2 + src/include/parser/parse_node.h | 1 + src/include/utils/partcache.h | 6 ++ src/test/regress/expected/create_table.out | 16 +--- src/test/regress/sql/create_table.sql | 5 +- 13 files changed, 142 insertions(+), 114 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 8c7b9619b8..2c11a5ad75 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -87,9 +87,9 @@ ALTER TABLE [ IF EXISTS ] name and partition_bound_spec is: -IN ( { numeric_literal | string_literal | NULL } [, ...] ) | -FROM ( { numeric_literal | string_literal | MINVALUE | MAXVALUE } [, ...] ) - TO ( { numeric_literal | string_literal | MINVALUE | MAXVALUE } [, ...] ) | +IN ( partition_bound_expr [, ...] ) | +FROM ( { partition_bound_expr | MINVALUE | MAXVALUE } [, ...] ) + TO ( { partition_bound_expr | MINVALUE | MAXVALUE } [, ...] ) | WITH ( MODULUS numeric_literal, REMAINDER numeric_literal ) and column_constraint is: diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index f2bd562d55..b7d5562345 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -86,9 +86,9 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI and partition_bound_spec is: -IN ( { numeric_literal | string_literal | NULL } [, ...] ) | -FROM ( { numeric_literal | string_literal | MINVALUE | MAXVALUE } [, ...] ) - TO ( { numeric_literal | string_literal | MINVALUE | MAXVALUE } [, ...] ) | +IN ( partition_bound_expr [, ...] ) | +FROM ( { partition_bound_expr | MINVALUE | MAXVALUE } [, ...] ) + TO ( { partition_bound_expr | MINVALUE | MAXVALUE } [, ...] ) | WITH ( MODULUS numeric_literal, REMAINDER numeric_literal ) index_parameters in UNIQUE, PRIMARY KEY, and EXCLUDE constraints are: @@ -275,10 +275,10 @@ WITH ( MODULUS numeric_literal, REM Each of the values specified in the partition_bound_spec is - a literal, NULL, MINVALUE, or - MAXVALUE. Each literal value must be either a - numeric constant that is coercible to the corresponding partition key - column's type, or a string literal that is valid input for that type. + any variable-free expression (subqueries, window functions, aggregate + functions, and set-returning functions are not allowed). The data type + of the partition bound expression must match the data type of the + corresponding partition key column. diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 505ae0af85..77ebb40ef2 100644 --- 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..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 part_elem %type part_params %type PartitionBoundSpec -%type partbound_datum PartitionRangeDatum -%type hash_partbound partbound_datum_list range_datum_list +%type hash_partbound %type 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,57 +2794,6 @@ 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 - { $$ = 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 @@ -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..4eb1c9f36e 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,11 @@ 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 Const *transformPartitionBoundValue(ParseState *pstate, Node *con, + const char *colName, Oid colType, int32 colTypmod, + Oid partCollation); /* @@ -3649,6 +3652,7 @@ transformPartitionBound(ParseState *pstate, Relation parent, char *colname; Oid coltype; int32 coltypmod; + Oid partcollation; if (spec->strategy != PARTITION_STRATEGY_LIST) ereport(ERROR, @@ -3668,17 +3672,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 +3731,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 +3746,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 +3764,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 +3783,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), @@ -3800,6 +3811,63 @@ transformPartitionBound(ParseState *pstate, Relation parent, } /* + * 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, transformExpr(), via transformPartitionBoundValue(), + * will error out with "column %s does not exist" message. + */ + } + else + { + prd->kind = PARTITION_RANGE_DATUM_VALUE; + prd->value = expr; + prd->location = exprLocation(expr); + } + + /* Use the existing List structure. */ + lfirst(lc) = prd; + } +} + +/* * validateInfiniteBounds * * Check that a MAXVALUE or MINVALUE specification in a partition bound is @@ -3843,13 +3911,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 +3934,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