From 0b39071a40778fb3ad364d71094ab384001f09bb Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" Date: Mon, 14 Sep 2026 12:41:10 -0700 Subject: [PATCH v2 1/2] 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 I moved the check out of transformColumnRef into a helper, and also called it from expandNSItemVars, which every expansion of a table's columns goes through. Expanding a composite value that isn't a table, like foo().*, makes no Vars, so it is still allowed. Author: Paul A. Jungwirth Backpatch-through: 14 --- src/backend/parser/parse_expr.c | 62 +++++++++++++--------- src/backend/parser/parse_relation.c | 5 ++ src/include/parser/parse_expr.h | 2 + src/test/regress/expected/alter_table.out | 8 +++ src/test/regress/expected/create_table.out | 23 ++++++++ src/test/regress/sql/alter_table.sql | 6 +++ src/test/regress/sql/create_table.sql | 12 +++++ 7 files changed, 93 insertions(+), 25 deletions(-) diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 728392f436f..e4c33163588 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -502,39 +502,24 @@ transformIndirection(ParseState *pstate, A_Indirection *ind) } /* - * Transform a ColumnRef. + * checkColumnRefAllowed + * Complain if a column reference is not allowed in the expression + * being transformed. * - * 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. Outside of any expression, + * as when expanding "*" in a SELECT list, they are always allowed. */ -static Node * -transformColumnRef(ParseState *pstate, ColumnRef *cref) +void +checkColumnRefAllowed(ParseState *pstate, 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) { case EXPR_KIND_NONE: - Assert(false); /* can't happen */ - break; case EXPR_KIND_OTHER: case EXPR_KIND_JOIN_ON: case EXPR_KIND_JOIN_USING: @@ -599,7 +584,34 @@ 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. */ + Assert(pstate->p_expr_kind != EXPR_KIND_NONE); + checkColumnRefAllowed(pstate, cref->location); /* * Give the PreParseColumnRefHook, if any, first shot. If it returns diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 8aeb3e219ac..140e40bf74e 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -25,6 +25,7 @@ #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" #include "parser/parse_enr.h" +#include "parser/parse_expr.h" #include "parser/parse_relation.h" #include "parser/parse_type.h" #include "parser/parsetree.h" @@ -3238,6 +3239,8 @@ expandTupleDesc(TupleDesc tupdesc, Alias *eref, int count, int offset, * for the non-dropped columns of the nsitem. * * The emitted Vars are marked with the given sublevels_up and location. + * They are column references, so we complain if those are forbidden in the + * expression being transformed. * * If colnames isn't NULL, a list of String items for the columns is stored * there; note that it's just a subset of the RTE's eref list, and hence @@ -3252,6 +3255,8 @@ expandNSItemVars(ParseState *pstate, ParseNamespaceItem *nsitem, int colindex; ListCell *lc; + checkColumnRefAllowed(pstate, location); + if (colnames) *colnames = NIL; colindex = 0; diff --git a/src/include/parser/parse_expr.h b/src/include/parser/parse_expr.h index e3de50c0d26..bbfe18a70c6 100644 --- a/src/include/parser/parse_expr.h +++ b/src/include/parser/parse_expr.h @@ -20,6 +20,8 @@ extern PGDLLIMPORT bool Transform_null_equals; extern Node *transformExpr(ParseState *pstate, Node *expr, ParseExprKind exprKind); +extern void checkColumnRefAllowed(ParseState *pstate, 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..e7788b78c16 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -128,6 +128,24 @@ 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, even one +-- that expands a composite value that is not a table +CREATE TABLE default_expr_rowexpr (id int DEFAULT (ROW(1)).f1); +DROP TABLE default_expr_rowexpr; +CREATE TABLE default_expr_rowexpr (id int DEFAULT (ROW((ROW(1, 2)).*)).f2); +INSERT INTO default_expr_rowexpr DEFAULT VALUES; +SELECT * FROM default_expr_rowexpr; + id +---- + 2 +(1 row) + +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 +464,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..df4ace75d78 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -81,6 +81,16 @@ 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, even one +-- that expands a composite value that is not a table +CREATE TABLE default_expr_rowexpr (id int DEFAULT (ROW(1)).f1); +DROP TABLE default_expr_rowexpr; +CREATE TABLE default_expr_rowexpr (id int DEFAULT (ROW((ROW(1, 2)).*)).f2); +INSERT INTO default_expr_rowexpr DEFAULT VALUES; +SELECT * FROM default_expr_rowexpr; +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 +326,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