On Thu, Mar 14, 2019 at 01:23:08PM +0900, Michael Paquier wrote: > I actually think that what you propose here makes more sense than what > HEAD does because the most inner expression gets evaluated first. > This for example generates the same error as on HEAD: > =# create table foo (a int default (avg(1))); > ERROR: 42803: aggregate functions are not allowed in DEFAULT expressions
I have been working on that, and in the case of a non-existing column
the patch would generate the following error on HEAD:
ERROR: 42703: column "non_existent" does not exist
But with the patch we get that:
ERROR: cannot use column reference in DEFAULT expression
Still I think that this looks right as we should not have any direct
column reference anyway, and it keeps the code more simple. So I have
added more tests to cover those grounds.
The docs of CREATE TABLE are actually wrong, right? It mentions that
"subqueries and cross-references to other columns in the current table
are not allowed", but cookDefault() rejects any kind of column
references anyway for default expressions, including references to the
column which uses the default expression (or we talk about generated
columns here still if I recall Peter Eisentraunt's patch correctly
generated columns don't allow references to the column using the
expression itself, which is logic by the way).
+ * transformExpr() should have already rejected column references,
+ * subqueries, aggregates, window functions, and SRFs, based on the
+ * EXPR_KIND_ for a default expression.
*/
I would have added an assertion here, perhaps an elog(). Same remark
for cookDefault(). The attached patch has an assertion.
CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES
IN (sum(a));
-ERROR: aggregate functions are not allowed in partition bound
+ERROR: cannot use column reference in partition bound expression
It would be nice to also test the case where an aggregate is
forbidden, so I have added a test with sum(1) instead of a column
reference.
We never actually tested in the tree the case of subqueries and SRFs
used in default expressions, so added.
The last patch you sent did not fix the original problem of the
thread. That was intentional from your side I guess to show your
point, still we are touching the same area of the code so I propose to
fix everything together, and to improve the test coverage for list and
range strategies. In order to achieve that, I have merged my previous
proposal into your patch, and added more tests. The new tests for the
range strategy reproduce the crash. The result is attached.
What do you think?
--
Michael
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index e94fe2c3b6..5fe16aa6f6 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -784,7 +784,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
The <literal>DEFAULT</literal> clause assigns a default data value for
the column whose column definition it appears within. The value
is any variable-free expression (subqueries and cross-references
- to other columns in the current table are not allowed). The
+ to any columns in the current table are not allowed). The
data type of the default expression must match the data type of the
column.
</para>
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index c7b5ff62f9..fc682e0b52 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2939,19 +2939,11 @@ cookDefault(ParseState *pstate,
expr = transformExpr(pstate, raw_default, EXPR_KIND_COLUMN_DEFAULT);
/*
- * Make sure default expr does not refer to any vars (we need this check
- * since the pstate includes the target table).
- */
- if (contain_var_clause(expr))
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
- errmsg("cannot use column references in default expression")));
-
- /*
- * transformExpr() should have already rejected subqueries, aggregates,
- * window functions, and SRFs, based on the EXPR_KIND_ for a default
- * expression.
+ * transformExpr() should have already rejected column references,
+ * subqueries, aggregates, window functions, and SRFs, based on the
+ * EXPR_KIND_ for a default expression.
*/
+ Assert(!contain_var_clause(expr));
/*
* Coerce the expression to the correct type and typmod, if given. This
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index e559353529..3e648dc8ef 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -520,6 +520,79 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
CRERR_WRONG_DB,
CRERR_TOO_MANY
} crerr = CRERR_NO_COLUMN;
+ const char *err;
+
+ /*
+ * Check to see if the column reference is in an invalid place within the
+ * query. We allow column references in most places, except in default
+ * expressions and partition bound expressions.
+ */
+ err = NULL;
+ switch (pstate->p_expr_kind)
+ {
+ case EXPR_KIND_NONE:
+ Assert(false); /* can't happen */
+ break;
+ case EXPR_KIND_OTHER:
+ case EXPR_KIND_JOIN_ON:
+ case EXPR_KIND_JOIN_USING:
+ case EXPR_KIND_FROM_SUBSELECT:
+ case EXPR_KIND_FROM_FUNCTION:
+ case EXPR_KIND_WHERE:
+ case EXPR_KIND_POLICY:
+ case EXPR_KIND_HAVING:
+ case EXPR_KIND_FILTER:
+ case EXPR_KIND_WINDOW_PARTITION:
+ case EXPR_KIND_WINDOW_ORDER:
+ case EXPR_KIND_WINDOW_FRAME_RANGE:
+ case EXPR_KIND_WINDOW_FRAME_ROWS:
+ case EXPR_KIND_WINDOW_FRAME_GROUPS:
+ case EXPR_KIND_SELECT_TARGET:
+ case EXPR_KIND_INSERT_TARGET:
+ case EXPR_KIND_UPDATE_SOURCE:
+ case EXPR_KIND_UPDATE_TARGET:
+ case EXPR_KIND_GROUP_BY:
+ case EXPR_KIND_ORDER_BY:
+ case EXPR_KIND_DISTINCT_ON:
+ case EXPR_KIND_LIMIT:
+ case EXPR_KIND_OFFSET:
+ case EXPR_KIND_RETURNING:
+ case EXPR_KIND_VALUES:
+ case EXPR_KIND_VALUES_SINGLE:
+ case EXPR_KIND_CHECK_CONSTRAINT:
+ case EXPR_KIND_DOMAIN_CHECK:
+ case EXPR_KIND_FUNCTION_DEFAULT:
+ case EXPR_KIND_INDEX_EXPRESSION:
+ case EXPR_KIND_INDEX_PREDICATE:
+ case EXPR_KIND_ALTER_COL_TRANSFORM:
+ case EXPR_KIND_EXECUTE_PARAMETER:
+ case EXPR_KIND_TRIGGER_WHEN:
+ case EXPR_KIND_PARTITION_EXPRESSION:
+ case EXPR_KIND_CALL_ARGUMENT:
+ case EXPR_KIND_COPY_WHERE:
+ /* okay */
+ break;
+
+ case EXPR_KIND_COLUMN_DEFAULT:
+ err = _("cannot use column reference in DEFAULT expression");
+ break;
+ case EXPR_KIND_PARTITION_BOUND:
+ err = _("cannot use column reference in partition bound expression");
+ break;
+
+ /*
+ * There is intentionally no default: case here, so that the
+ * compiler will warn if we add a new ParseExprKind without
+ * extending this switch. If we do see an unrecognized value at
+ * runtime, the behavior will be the same as for EXPR_KIND_OTHER,
+ * which is sane anyway.
+ */
+ }
+ if (err)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg_internal("%s", err),
+ parser_errposition(pstate, cref->location)));
/*
* Give the PreParseColumnRefHook, if any, first shot. If it returns
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index a37d1f18be..d38ad1ed62 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -3750,8 +3750,16 @@ transformPartitionRangeBounds(ParseState *pstate, List *blist,
IsA(linitial(cref->fields), String))
cname = strVal(linitial(cref->fields));
- Assert(cname != NULL);
- if (strcmp("minvalue", cname) == 0)
+ if (cname == NULL)
+ {
+ /*
+ * No field names have been found, meaning that there
+ * is not much to do with special value handling. Instead
+ * let the expression transformation handle any errors and
+ * limitations.
+ */
+ }
+ else if (strcmp("minvalue", cname) == 0)
{
prd = makeNode(PartitionRangeDatum);
prd->kind = PARTITION_RANGE_DATUM_MINVALUE;
@@ -3903,6 +3911,13 @@ transformPartitionBoundValue(ParseState *pstate, Node *val,
COERCE_IMPLICIT_CAST,
-1);
+ /*
+ * transformExpr() should have already rejected column references,
+ * subqueries, aggregates, window functions, and SRFs, based on the
+ * EXPR_KIND_ for a default expression.
+ */
+ Assert(!contain_var_clause(value));
+
if (value == NULL)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
@@ -3914,13 +3929,6 @@ transformPartitionBoundValue(ParseState *pstate, Node *val,
if (!IsA(value, Const))
value = (Node *) expression_planner((Expr *) value);
- /* Make sure the expression does not refer to any vars. */
- if (contain_var_clause(value))
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
- errmsg("cannot use column references in partition bound expression"),
- parser_errposition(pstate, exprLocation(value))));
-
/*
* Evaluate the expression, assigning the partition key's collation to the
* resulting Const expression.
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index d51e547278..d3a7a8c21b 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -297,6 +297,38 @@ ERROR: tables declared WITH OIDS are not supported
-- but explicitly not adding oids is still supported
CREATE TEMP TABLE withoutoid() WITHOUT OIDS; DROP TABLE withoutoid;
CREATE TEMP TABLE withoutoid() WITH (oids = false); DROP TABLE withoutoid;
+-- check restriction with default expressions
+-- invalid use of column reference in default expressions
+CREATE TABLE default_expr_column (id int DEFAULT (id));
+ERROR: cannot use column reference in DEFAULT expression
+LINE 1: CREATE TABLE default_expr_column (id int DEFAULT (id));
+ ^
+CREATE TABLE default_expr_column (id int DEFAULT (bar.id));
+ERROR: cannot use column reference in DEFAULT expression
+LINE 1: CREATE TABLE default_expr_column (id int DEFAULT (bar.id));
+ ^
+CREATE TABLE default_expr_agg_column (id int DEFAULT (avg(id)));
+ERROR: cannot use column reference in DEFAULT expression
+LINE 1: ...TE TABLE default_expr_agg_column (id int DEFAULT (avg(id)));
+ ^
+-- invalid column definition
+CREATE TABLE default_expr_non_column (a int DEFAULT (avg(non_existent)));
+ERROR: cannot use column reference in DEFAULT expression
+LINE 1: ...TABLE default_expr_non_column (a int DEFAULT (avg(non_existe...
+ ^
+-- invalid use of aggregate in default expression
+CREATE TABLE default_expr_agg (a int DEFAULT (avg(1)));
+ERROR: aggregate functions are not allowed in DEFAULT expressions
+LINE 1: CREATE TABLE default_expr_agg (a int DEFAULT (avg(1)));
+ ^
+CREATE TABLE default_expr_agg (a int DEFAULT (select 1));
+ERROR: cannot use subquery in DEFAULT expression
+LINE 1: CREATE TABLE default_expr_agg (a int DEFAULT (select 1));
+ ^
+CREATE TABLE default_expr_agg (a int DEFAULT (generate_series(1,3)));
+ERROR: set-returning functions are not allowed in DEFAULT expressions
+LINE 1: CREATE TABLE default_expr_agg (a int DEFAULT (generate_serie...
+ ^
--
-- Partitioned tables
--
@@ -489,18 +521,34 @@ Partitions: part_null FOR VALUES IN (NULL),
part_p2 FOR VALUES IN (2),
part_p3 FOR VALUES IN (3)
--- forbidden expressions for partition bound
+-- forbidden expressions for partition bound with list partitioned table
CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (somename);
-ERROR: column "somename" does not exist
+ERROR: cannot use column reference in partition bound expression
LINE 1: ...expr_fail PARTITION OF list_parted FOR VALUES IN (somename);
^
+CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (somename.somename);
+ERROR: cannot use column reference in partition bound expression
+LINE 1: ...expr_fail PARTITION OF list_parted FOR VALUES IN (somename.s...
+ ^
CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (a);
-ERROR: cannot use column references in partition bound expression
+ERROR: cannot use column reference in partition bound expression
LINE 1: ..._bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (a);
^
+CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (a.a);
+ERROR: cannot use column reference in partition bound expression
+LINE 1: ...ogus_expr_fail PARTITION OF list_parted FOR VALUES IN (a.a);
+ ^
CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (sum(a));
-ERROR: aggregate functions are not allowed in partition bound
+ERROR: cannot use column reference in partition bound expression
LINE 1: ...s_expr_fail PARTITION OF list_parted FOR VALUES IN (sum(a));
+ ^
+CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (sum(somename));
+ERROR: cannot use column reference in partition bound expression
+LINE 1: ..._fail PARTITION OF list_parted FOR VALUES IN (sum(somename))...
+ ^
+CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (sum(1));
+ERROR: aggregate functions are not allowed in partition bound
+LINE 1: ...s_expr_fail PARTITION OF list_parted FOR VALUES IN (sum(1));
^
CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN ((select 1));
ERROR: cannot use subquery in partition bound
@@ -558,6 +606,52 @@ DROP TABLE bigintp;
CREATE TABLE range_parted (
a date
) PARTITION BY RANGE (a);
+-- forbidden expressions for partition bounds with range partitioned table
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (somename) TO ('2019-01-01');
+ERROR: cannot use column reference in partition bound expression
+LINE 2: FOR VALUES FROM (somename) TO ('2019-01-01');
+ ^
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (somename.somename) TO ('2019-01-01');
+ERROR: cannot use column reference in partition bound expression
+LINE 2: FOR VALUES FROM (somename.somename) TO ('2019-01-01');
+ ^
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (a) TO ('2019-01-01');
+ERROR: cannot use column reference in partition bound expression
+LINE 2: FOR VALUES FROM (a) TO ('2019-01-01');
+ ^
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (a.a) TO ('2019-01-01');
+ERROR: cannot use column reference in partition bound expression
+LINE 2: FOR VALUES FROM (a.a) TO ('2019-01-01');
+ ^
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (sum(a)) TO ('2019-01-01');
+ERROR: cannot use column reference in partition bound expression
+LINE 2: FOR VALUES FROM (sum(a)) TO ('2019-01-01');
+ ^
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (sum(somename)) TO ('2019-01-01');
+ERROR: cannot use column reference in partition bound expression
+LINE 2: FOR VALUES FROM (sum(somename)) TO ('2019-01-01');
+ ^
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (sum(1)) TO ('2019-01-01');
+ERROR: aggregate functions are not allowed in partition bound
+LINE 2: FOR VALUES FROM (sum(1)) TO ('2019-01-01');
+ ^
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM ((select 1)) TO ('2019-01-01');
+ERROR: cannot use subquery in partition bound
+LINE 2: FOR VALUES FROM ((select 1)) TO ('2019-01-01');
+ ^
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (generate_series(1, 3)) TO ('2019-01-01');
+ERROR: set-returning functions are not allowed in partition bound
+LINE 2: FOR VALUES FROM (generate_series(1, 3)) TO ('2019-01-01');
+ ^
-- trying to specify list for range partitioned table
CREATE TABLE fail_part PARTITION OF range_parted FOR VALUES IN ('a');
ERROR: invalid bound specification for a range partition
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 4091c19cf0..b0704d90af 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -304,6 +304,18 @@ CREATE TABLE withoid() WITH (oids = true);
CREATE TEMP TABLE withoutoid() WITHOUT OIDS; DROP TABLE withoutoid;
CREATE TEMP TABLE withoutoid() WITH (oids = false); DROP TABLE withoutoid;
+-- check restriction with default expressions
+-- invalid use of column reference in default expressions
+CREATE TABLE default_expr_column (id int DEFAULT (id));
+CREATE TABLE default_expr_column (id int DEFAULT (bar.id));
+CREATE TABLE default_expr_agg_column (id int DEFAULT (avg(id)));
+-- invalid column definition
+CREATE TABLE default_expr_non_column (a int DEFAULT (avg(non_existent)));
+-- invalid use of aggregate in default expression
+CREATE TABLE default_expr_agg (a int DEFAULT (avg(1)));
+CREATE TABLE default_expr_agg (a int DEFAULT (select 1));
+CREATE TABLE default_expr_agg (a int DEFAULT (generate_series(1,3)));
+
--
-- Partitioned tables
--
@@ -450,10 +462,14 @@ CREATE TABLE part_p3 PARTITION OF list_parted FOR VALUES IN ((2+1));
CREATE TABLE part_null PARTITION OF list_parted FOR VALUES IN (null);
\d+ list_parted
--- forbidden expressions for partition bound
+-- forbidden expressions for partition bound with list partitioned table
CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (somename);
+CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (somename.somename);
CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (a);
+CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (a.a);
CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (sum(a));
+CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (sum(somename));
+CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (sum(1));
CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN ((select 1));
CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (generate_series(4, 6));
@@ -497,6 +513,26 @@ CREATE TABLE range_parted (
a date
) PARTITION BY RANGE (a);
+-- forbidden expressions for partition bounds with range partitioned table
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (somename) TO ('2019-01-01');
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (somename.somename) TO ('2019-01-01');
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (a) TO ('2019-01-01');
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (a.a) TO ('2019-01-01');
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (sum(a)) TO ('2019-01-01');
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (sum(somename)) TO ('2019-01-01');
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (sum(1)) TO ('2019-01-01');
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM ((select 1)) TO ('2019-01-01');
+CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted
+ FOR VALUES FROM (generate_series(1, 3)) TO ('2019-01-01');
+
-- trying to specify list for range partitioned table
CREATE TABLE fail_part PARTITION OF range_parted FOR VALUES IN ('a');
-- trying to specify modulus and remainder for range partitioned table
signature.asc
Description: PGP signature
