From 2bff9ab8f64582b5e82c1a1ad897a9cfe69b8242 Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" Date: Mon, 14 Sep 2026 12:41:10 -0700 Subject: [PATCH v1] Reject whole-row references where column references are forbidden There are a couple expression kinds that forbid column references (partition bounds and DEFAULTs), but you could still inject one by going through a whole-row reference: ExpandColumnRefStar wasn't doing the same check as transformColumnRef. That allowed these errors: -- crashes ALTER TABLE t ADD COLUMN b int DEFAULT (ROW(t.*)).f1; -- crashes CREATE TABLE p1 PARTITION OF p FOR VALUES IN ((ROW(p1.*)).f1); -- wrongly accepted CREATE TABLE t (a int, b int DEFAULT (ROW(t.*)).f1); ALTER TABLE t ALTER COLUMN b SET DEFAULT (ROW(t.*)).f1; INSERT INTO t (a) VALUES (1); -- crashes here instead To help the functions stay in sync, we factor out the check into a shared helper. Actually instead of calling it from ExpandColumnRefStar, I'm calling it from (one of) its callers, since it isn't always necessary. Note we can't use pstate->p_expr_kind, since that isn't set if we didn't come through transformExpr. Author: Paul A. Jungwirth Backpatch-through: 14 --- src/backend/parser/parse_expr.c | 60 +++++++++++++--------- src/backend/parser/parse_target.c | 6 ++- src/include/parser/parse_expr.h | 3 ++ src/test/regress/expected/alter_table.out | 8 +++ src/test/regress/expected/create_table.out | 13 +++++ src/test/regress/sql/alter_table.sql | 6 +++ src/test/regress/sql/create_table.sql | 7 +++ 7 files changed, 78 insertions(+), 25 deletions(-) diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 728392f436f..e69a1859d49 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -502,35 +502,21 @@ transformIndirection(ParseState *pstate, A_Indirection *ind) } /* - * Transform a ColumnRef. + * checkColumnRefAllowed + * Complain if a column reference is not allowed in the given kind of + * expression. * - * If you find yourself changing this code, see also ExpandColumnRefStar. + * We allow column references in most places, except in a few kinds of + * expression that must produce the same value for every row, namely default + * expressions and partition bound expressions. */ -static Node * -transformColumnRef(ParseState *pstate, ColumnRef *cref) +void +checkColumnRefAllowed(ParseState *pstate, ParseExprKind exprKind, int location) { - Node *node = NULL; - char *nspname = NULL; - char *relname = NULL; - char *colname = NULL; - ParseNamespaceItem *nsitem; - int levels_up; - enum - { - CRERR_NO_COLUMN, - CRERR_NO_RTE, - 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) + switch (exprKind) { case EXPR_KIND_NONE: Assert(false); /* can't happen */ @@ -599,7 +585,33 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg_internal("%s", err), - parser_errposition(pstate, cref->location))); + parser_errposition(pstate, location))); +} + +/* + * Transform a ColumnRef. + * + * If you find yourself changing this code, see also ExpandColumnRefStar. + */ +static Node * +transformColumnRef(ParseState *pstate, ColumnRef *cref) +{ + Node *node = NULL; + char *nspname = NULL; + char *relname = NULL; + char *colname = NULL; + ParseNamespaceItem *nsitem; + int levels_up; + enum + { + CRERR_NO_COLUMN, + CRERR_NO_RTE, + CRERR_WRONG_DB, + CRERR_TOO_MANY + } crerr = CRERR_NO_COLUMN; + + /* Check to see if the column reference is in an invalid place. */ + checkColumnRefAllowed(pstate, pstate->p_expr_kind, cref->location); /* * Give the PreParseColumnRefHook, if any, first shot. If it returns diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index 728936b502e..cb882cf5e12 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -237,7 +237,11 @@ transformExpressionList(ParseState *pstate, List *exprlist, if (IsA(llast(cref->fields), A_Star)) { - /* It is something.*, expand into multiple items */ + /* + * It is something.*, expand into multiple items, as long as + * column references are allowed. + */ + checkColumnRefAllowed(pstate, exprKind, cref->location); result = list_concat(result, ExpandColumnRefStar(pstate, cref, false)); diff --git a/src/include/parser/parse_expr.h b/src/include/parser/parse_expr.h index e3de50c0d26..c9e193c2b88 100644 --- a/src/include/parser/parse_expr.h +++ b/src/include/parser/parse_expr.h @@ -20,6 +20,9 @@ extern PGDLLIMPORT bool Transform_null_equals; extern Node *transformExpr(ParseState *pstate, Node *expr, ParseExprKind exprKind); +extern void checkColumnRefAllowed(ParseState *pstate, ParseExprKind exprKind, + int location); + extern const char *ParseExprKindName(ParseExprKind exprKind); #endif /* PARSE_EXPR_H */ diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 20707c186b3..6983e60a99e 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1351,6 +1351,14 @@ alter table def_test alter column c2 set default 20; -- set defaults on a non-existent column: this should fail alter table def_test alter column c3 set default 30; ERROR: column "c3" of relation "def_test" does not exist +-- a default may not reference a column, not even through a whole-row +-- reference: these should all fail +alter table def_test alter column c1 set default def_test.c1; +ERROR: cannot use column reference in DEFAULT expression +alter table def_test alter column c1 set default (ROW(def_test.*)).f1; +ERROR: cannot use column reference in DEFAULT expression +alter table def_test add column c4 int default (ROW(def_test.*)).f1; +ERROR: cannot use column reference in DEFAULT expression -- set defaults on views: we need to create a view, add a rule -- to allow insertions into it, and then alter the view to add -- a default diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 2df8761ae8e..5922cb869da 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -128,6 +128,14 @@ 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))); ^ +-- a whole-row reference is a column reference too +CREATE TABLE default_expr_wholerow (id int DEFAULT (ROW(default_expr_wholerow.*)).f1); +ERROR: cannot use column reference in DEFAULT expression +LINE 1: ... TABLE default_expr_wholerow (id int DEFAULT (ROW(default_ex... + ^ +-- but a row constructor with no column reference in it is fine +CREATE TABLE default_expr_rowexpr (id int DEFAULT (ROW(1)).f1); +DROP TABLE default_expr_rowexpr; -- invalid column definition CREATE TABLE default_expr_non_column (a int DEFAULT (avg(non_existent))); ERROR: cannot use column reference in DEFAULT expression @@ -446,6 +454,11 @@ CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (somena ERROR: cannot use column reference in partition bound expression LINE 1: ...expr_fail PARTITION OF list_parted FOR VALUES IN (somename.s... ^ +-- a whole-row reference to the partition being created is a column reference +CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN ((ROW(part_bogus_expr_fail.*)).f1); +ERROR: cannot use column reference in partition bound expression +LINE 1: ...fail PARTITION OF list_parted FOR VALUES IN ((ROW(part_bogus... + ^ CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (a); ERROR: cannot use column reference in partition bound expression LINE 1: ..._bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (a); diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 18f3fdac57a..af84b7a02d3 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -987,6 +987,12 @@ alter table def_test alter column c2 set default 20; -- set defaults on a non-existent column: this should fail alter table def_test alter column c3 set default 30; +-- a default may not reference a column, not even through a whole-row +-- reference: these should all fail +alter table def_test alter column c1 set default def_test.c1; +alter table def_test alter column c1 set default (ROW(def_test.*)).f1; +alter table def_test add column c4 int default (ROW(def_test.*)).f1; + -- set defaults on views: we need to create a view, add a rule -- to allow insertions into it, and then alter the view to add -- a default diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index 80e424e6bda..01667403cc2 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -81,6 +81,11 @@ DROP TABLE relation_filenode_check; 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))); +-- a whole-row reference is a column reference too +CREATE TABLE default_expr_wholerow (id int DEFAULT (ROW(default_expr_wholerow.*)).f1); +-- but a row constructor with no column reference in it is fine +CREATE TABLE default_expr_rowexpr (id int DEFAULT (ROW(1)).f1); +DROP TABLE default_expr_rowexpr; -- invalid column definition CREATE TABLE default_expr_non_column (a int DEFAULT (avg(non_existent))); -- invalid use of aggregate @@ -316,6 +321,8 @@ CREATE TABLE part_null PARTITION OF list_parted 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); CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (somename.somename); +-- a whole-row reference to the partition being created is a column reference +CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN ((ROW(part_bogus_expr_fail.*)).f1); 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 (sum(a)); CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted FOR VALUES IN (sum(somename)); -- 2.47.3