From 38d0a90035f7de3f6eac289089b8c20e4e395f76 Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" Date: Mon, 2 Nov 2026 14:50:56 -0800 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 each caller of expandNSItemVars that can put the Vars in an expression. On master the call is inside expandNSItemVars, but here that function has no ParseState, and adding one would change an exported signature. 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_coerce.c | 2 + src/backend/parser/parse_expr.c | 62 +++++++++++++--------- src/backend/parser/parse_relation.c | 4 ++ src/backend/parser/parse_target.c | 1 + 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 +++++ 9 files changed, 95 insertions(+), 25 deletions(-) diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c index cef31b9a5fe..d0521124aa6 100644 --- a/src/backend/parser/parse_coerce.c +++ b/src/backend/parser/parse_coerce.c @@ -22,6 +22,7 @@ #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" #include "parser/parse_coerce.h" +#include "parser/parse_expr.h" #include "parser/parse_relation.h" #include "parser/parse_type.h" #include "utils/builtins.h" @@ -1046,6 +1047,7 @@ coerce_record_to_complex(ParseState *pstate, Node *node, ParseNamespaceItem *nsitem; nsitem = GetNSItemByRangeTablePosn(pstate, rtindex, sublevels_up); + checkColumnRefAllowed(pstate, vlocation); args = expandNSItemVars(nsitem, sublevels_up, vlocation, NULL); } else diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 2e0a5b797a1..95a57b13776 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -431,39 +431,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: @@ -527,7 +512,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 e0066830299..860a5bf0e60 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -27,6 +27,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" @@ -3098,6 +3099,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 a caller putting them in an expression must + * first check with checkColumnRefAllowed that those are allowed there. * * 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 @@ -3174,6 +3177,7 @@ expandNSItemAttrs(ParseState *pstate, ParseNamespaceItem *nsitem, *var; List *te_list = NIL; + checkColumnRefAllowed(pstate, location); vars = expandNSItemVars(nsitem, sublevels_up, location, &names); /* diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index 4d6d88c33a4..e0dd6705a63 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -1393,6 +1393,7 @@ ExpandSingleTable(ParseState *pstate, ParseNamespaceItem *nsitem, List *vars; ListCell *l; + checkColumnRefAllowed(pstate, location); vars = expandNSItemVars(nsitem, sublevels_up, location, NULL); /* diff --git a/src/include/parser/parse_expr.h b/src/include/parser/parse_expr.h index c8e5c57b430..e6527ad07d6 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 c0ea4c9ec1e..fb51a75c37e 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1239,6 +1239,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 4806be67706..c3e40b0a340 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -118,6 +118,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 @@ -435,6 +453,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 2bbabd4b528..c7bf5fe130a 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -950,6 +950,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 b761b63daf1..eb30e2eecd3 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -75,6 +75,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 @@ -309,6 +319,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