Hi Hackers,
Here is a fix to some crashes when using a wholerow reference to sneak
in a ColumnRef where it shouldn't be allowed. This was originally
reported in [0] by Andres while reviewing FOR PORTION OF, but it is
actually a problem on master, going back at least to v14. You can
reach a crash like so:
-- 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
Today, we forbid some EXPR_KINDs in transformColumnRef, but we need to
do that in (or near) ExpandColumnRefStar as well. I factored out the
check to checkColumnRefAllowed to avoid future inconsistencies.
[0]
https://www.postgresql.org/message-id/yfers7d2kizq6ockz4nu7e7avq3gj5anpd7ilxkazyvicv4qgy@3cesow2bbhnx
Yours,
--
Paul ~{:-)
[email protected]
From 2bff9ab8f64582b5e82c1a1ad897a9cfe69b8242 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email protected]>
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 <[email protected]>
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
