At Wed, 11 Apr 2018 11:27:17 +0900, Amit Langote
<langote_amit...@lab.ntt.co.jp> wrote in
<1810b14f-3cd7-aff5-8358-c225c0231...@lab.ntt.co.jp>
> On 2018/04/11 10:44, Tom Lane wrote:
> > Kyotaro HORIGUCHI <horiguchi.kyot...@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;
