public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Postgres hackers <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: jian he <[email protected]>
Subject: Fix SET EXPRESSION for virtual columns with whole-row dependencies
Date: Tue, 9 Jun 2026 14:22:32 +0800
Message-ID: <[email protected]> (raw)
Hi,
While testing “[f80bedd52] Allow ALTER COLUMN SET EXPRESSION on virtual columns”, I found that the feature missed handling whole-row check constraints.
Here is a repro:
```
evantest=# create table t(
evantest(# a int,
evantest(# b int generated always as (a*2) virtual,
evantest(# constraint row_c check (t is not null)
evantest(# );
CREATE TABLE
evantest=# insert into t(a) values(1);
INSERT 0 1
evantest=# alter table t alter b set expression as (nullif(a, 1));
ALTER TABLE
evantest=# select * from t;
a | b
---+---
1 |
(1 row)
```
The ALTER TABLE should fail, because it makes b become NULL, which breaks the constraint row_c.
For comparison, if we use the nullif expression at table creation time, we cannot insert a row with a = 1:
```
evantest=# create table t1(
evantest(# a int,
evantest(# b int generated always as (nullif(a,1)) virtual,
evantest(# constraint row_c check (t1 is not null)
evantest(# );
CREATE TABLE
evantest=# insert into t1(a) values(1);
ERROR: new row for relation "t1" violates check constraint "row_c"
DETAIL: Failing row contains (1, virtual).
```
I think the problem is that ATExecSetExpression() only calls RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName); to find dependent objects. In this repro, attnum is 2, which is b’s attnum, and colName is b, so it doesn’t find the whole-row constraint row_c. The same problem applies to whole-row indexes as well.
Since RememberAllDependentForRebuilding() is only used by AT_AlterColumnType and AT_SetExpression, I enhanced it to handle attnum == 0 for AT_SetExpression, so that whole-row dependent constraints and indexes are remembered for a virtual generated column.
See the attached patch for details. With the fix, rerunning the repro makes ALTER TABLE SET EXPRESSION fail as expected:
```
evantest=# alter table t alter b set expression as (nullif(a, 1));
ERROR: check constraint "row_c" of relation "t" is violated by some row
```
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v1-0001-Fix-SET-EXPRESSION-with-whole-row-virtual-column-.patch (7.5K, ../[email protected]/2-v1-0001-Fix-SET-EXPRESSION-with-whole-row-virtual-column-.patch)
download | inline diff:
From 54088e2c329dfed17e861cbe8d397e5b707485ed Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Tue, 9 Jun 2026 14:05:00 +0800
Subject: [PATCH v1] Fix SET EXPRESSION with whole-row virtual column
dependencies
ALTER COLUMN SET EXPRESSION on a virtual generated column only looked for
dependencies on the column's attnum. Whole-row expressions, such as
"rel IS NOT NULL", depend on the relation as a whole instead, but they can
still observe the virtual column's computed value.
Teach the SET EXPRESSION path to also scan whole-relation dependencies for
virtual generated columns. This lets whole-row CHECK constraints be
revalidated and whole-row expression indexes be rebuilt after the expression
changes.
Add regression coverage for a whole-row CHECK constraint, a whole-row
expression index, and an ordinary index that should not be rebuilt.
Author: Chao Li <[email protected]>
---
src/backend/commands/tablecmds.c | 40 +++++++++++++++++--
.../regress/expected/generated_virtual.out | 19 +++++++++
src/test/regress/sql/generated_virtual.sql | 8 ++++
3 files changed, 64 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a33e22e8e61..594aabfa295 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8765,6 +8765,15 @@ ATExecSetExpression(AlteredTableInfo *tab, Relation rel, const char *colName,
* and record enough information to let us recreate the objects.
*/
RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName);
+ if (attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+ {
+ /*
+ * Whole-row expressions, such as "rel IS NOT NULL", depend on the
+ * relation as a whole rather than on the virtual generated column's
+ * attnum, but they can observe the column's new computed value.
+ */
+ RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, 0, NULL);
+ }
/*
* Drop the dependency records of the GENERATED expression, in particular
@@ -15293,7 +15302,9 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
/*
* Subroutine for ATExecAlterColumnType and ATExecSetExpression: Find everything
* that depends on the column (constraints, indexes, etc), and record enough
- * information to let us recreate the objects.
+ * information to let us recreate the objects. For virtual generated columns,
+ * ATExecSetExpression can also pass attnum == 0 to find whole-row expression
+ * dependencies that observe the column's computed value.
*/
static void
RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype,
@@ -15305,6 +15316,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype,
HeapTuple depTup;
Assert(subtype == AT_AlterColumnType || subtype == AT_SetExpression);
+ Assert(attnum != 0 || subtype == AT_SetExpression);
depRel = table_open(DependRelationId, RowExclusiveLock);
@@ -15355,6 +15367,15 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype,
}
else
{
+ /*
+ * In a whole-relation scan for SET EXPRESSION, ignore
+ * relation dependencies that are not indexes. Objects
+ * with expressions that need action, such as
+ * constraints, are handled by their own cases.
+ */
+ if (attnum == 0)
+ break;
+
/* Not expecting any other direct dependencies... */
elog(ERROR, "unexpected object depending on column: %s",
getObjectDescription(&foundObject, false));
@@ -15364,7 +15385,14 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype,
case ConstraintRelationId:
Assert(foundObject.objectSubId == 0);
- RememberConstraintForRebuilding(foundObject.objectId, tab);
+ if (attnum == 0)
+ {
+ /* Whole-row CHECK constraints may need to be revalidated */
+ if (get_constraint_type(foundObject.objectId) == CONSTRAINT_CHECK)
+ RememberConstraintForRebuilding(foundObject.objectId, tab);
+ }
+ else
+ RememberConstraintForRebuilding(foundObject.objectId, tab);
break;
case ProcedureRelationId:
@@ -15505,8 +15533,14 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype,
/*
* We don't expect any other sorts of objects to depend on a
- * column.
+ * column. A whole-relation scan can find the relation's row
+ * type, which doesn't need rebuilding for SET EXPRESSION.
*/
+ if (attnum == 0 &&
+ foundObject.classId == TypeRelationId &&
+ get_typ_typrelid(foundObject.objectId) == RelationGetRelid(rel))
+ continue;
+
elog(ERROR, "unexpected object depending on column: %s",
getObjectDescription(&foundObject, false));
break;
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index 24d5dbf46ca..625b4025828 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -694,6 +694,25 @@ INSERT INTO gtest20c VALUES (1); -- ok
INSERT INTO gtest20c VALUES (NULL); -- fails
ERROR: new row for relation "gtest20c" violates check constraint "whole_row_check"
DETAIL: Failing row contains (null, virtual).
+ALTER TABLE gtest20c ALTER COLUMN b SET EXPRESSION AS (nullif(a, 1)); -- fails
+ERROR: check constraint "whole_row_check" of relation "gtest20c" is violated by some row
+CREATE INDEX gtest20c_expr_idx ON gtest20c ((gtest20c IS NOT NULL));
+CREATE INDEX gtest20c_a_idx ON gtest20c (a);
+SELECT pg_relation_filenode('gtest20c_expr_idx') AS gtest20c_expr_idx_filenode \gset
+SELECT pg_relation_filenode('gtest20c_a_idx') AS gtest20c_a_idx_filenode \gset
+ALTER TABLE gtest20c ALTER COLUMN b SET EXPRESSION AS (a * 3); -- ok, rebuilds index
+SELECT pg_relation_filenode('gtest20c_expr_idx') <> :gtest20c_expr_idx_filenode AS whole_row_index_rebuilt;
+ whole_row_index_rebuilt
+-------------------------
+ t
+(1 row)
+
+SELECT pg_relation_filenode('gtest20c_a_idx') = :gtest20c_a_idx_filenode AS ordinary_index_not_rebuilt;
+ ordinary_index_not_rebuilt
+----------------------------
+ t
+(1 row)
+
-- not-null constraints
CREATE TABLE gtest21a (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL NOT NULL);
INSERT INTO gtest21a (a) VALUES (1); -- ok
diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index 9c2bb6590b3..14fa47b6173 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -347,6 +347,14 @@ CREATE TABLE gtest20c (a int, b int GENERATED ALWAYS AS (a * 2) VIRTUAL);
ALTER TABLE gtest20c ADD CONSTRAINT whole_row_check CHECK (gtest20c IS NOT NULL);
INSERT INTO gtest20c VALUES (1); -- ok
INSERT INTO gtest20c VALUES (NULL); -- fails
+ALTER TABLE gtest20c ALTER COLUMN b SET EXPRESSION AS (nullif(a, 1)); -- fails
+CREATE INDEX gtest20c_expr_idx ON gtest20c ((gtest20c IS NOT NULL));
+CREATE INDEX gtest20c_a_idx ON gtest20c (a);
+SELECT pg_relation_filenode('gtest20c_expr_idx') AS gtest20c_expr_idx_filenode \gset
+SELECT pg_relation_filenode('gtest20c_a_idx') AS gtest20c_a_idx_filenode \gset
+ALTER TABLE gtest20c ALTER COLUMN b SET EXPRESSION AS (a * 3); -- ok, rebuilds index
+SELECT pg_relation_filenode('gtest20c_expr_idx') <> :gtest20c_expr_idx_filenode AS whole_row_index_rebuilt;
+SELECT pg_relation_filenode('gtest20c_a_idx') = :gtest20c_a_idx_filenode AS ordinary_index_not_rebuilt;
-- not-null constraints
CREATE TABLE gtest21a (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL NOT NULL);
--
2.50.1 (Apple Git-155)
view thread (9+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Fix SET EXPRESSION for virtual columns with whole-row dependencies
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox