From 27e064b15fc7d0ebdb7f31e5f901117aba84af28 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 22 Mar 2019 14:06:16 +0900 Subject: [PATCH 2/2] Rework error messages for incorrect column references --- doc/src/sgml/ref/create_table.sgml | 2 +- src/backend/catalog/heap.c | 16 ++--- src/backend/parser/parse_expr.c | 73 ++++++++++++++++++++++ src/backend/parser/parse_utilcmd.c | 12 ++-- src/test/regress/expected/create_table.out | 63 ++++++++++++++----- src/test/regress/sql/create_table.sql | 14 +++++ 6 files changed, 146 insertions(+), 34 deletions(-) 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 numeric_literal, REM The DEFAULT 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. 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 ee129ba18f..34eec1d5dd 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -3922,12 +3922,12 @@ 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)))); + /* + * 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)); /* * Evaluate the expression, assigning the partition key's collation to the diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 130dccb241..479b64eee0 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -297,6 +297,40 @@ 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 +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))); + ^ +-- invalid use of subquery +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)); + ^ +-- invalid use of set-returning function +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 -- @@ -491,27 +525,27 @@ Partitions: part_null FOR VALUES IN (NULL), -- 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: missing FROM-clause entry for table "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: missing FROM-clause entry for table "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: column "somename" does not exist +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)); @@ -577,33 +611,32 @@ CREATE TABLE range_parted ( -- 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: column "somename" does not exist +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: missing FROM-clause entry for table "somename" +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 references in partition bound expression +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: missing FROM-clause entry for table "a" +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: function sum(date) does not exist +ERROR: cannot use column reference in partition bound expression LINE 2: FOR VALUES FROM (sum(a)) TO ('2019-01-01'); - ^ -HINT: No function matches the given name and argument types. You might need to add explicit type casts. + ^ CREATE TABLE part_bogus_expr_fail PARTITION OF range_parted FOR VALUES FROM (sum(somename)) TO ('2019-01-01'); -ERROR: column "somename" does not exist +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 diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index fac6ffe947..9897470994 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -304,6 +304,20 @@ 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 +CREATE TABLE default_expr_agg (a int DEFAULT (avg(1))); +-- invalid use of subquery +CREATE TABLE default_expr_agg (a int DEFAULT (select 1)); +-- invalid use of set-returning function +CREATE TABLE default_expr_agg (a int DEFAULT (generate_series(1,3))); + -- -- Partitioned tables -- -- 2.20.1