Subject: Fix SET EXPRESSION for virtual columns with whole-row dependencies

From: Chao Li <[email-masked]>
Date: Tue, 9 Jun 2026 14:22:32 +0800

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/

---

From: jian he <[email-masked]>
Date: Tue, 9 Jun 2026 15:23:24 +0800

> The ALTER TABLE should fail, because it makes b become NULL, which breaks the constraint row_c.

Hi.

In case you are wondering whole-row objects:
ALTER TABLE DROP COLUMN and ALTER COLUMN SET DATA TYPE are covered by
https://commitfest.postgresql.org/patch/5988 and
https://commitfest.postgresql.org/patch/6055.

Meanwhile, ALTER TABLE SET EXPRESSION is being handled over in
https://commitfest.postgresql.org/patch/6755

---

From: Chao Li <[email-masked]>
Date: Tue, 9 Jun 2026 15:28:18 +0800

> On Jun 9, 2026, at 15:23, jian he <[email-masked]> wrote:
> 

Thanks for pointing out that. I will review 6755.

Best regards,

---

From: Chao Li <[email-masked]>
Date: Wed, 10 Jun 2026 17:05:30 +0800

> On Jun 9, 2026, at 15:28, Chao Li <[email-masked]> wrote:
> 
> 
> 

After reading the other implementation in [1], I realized that I had missed the partial-index case, so I added coverage for that.

I am still sending an updated version of this patch because my implementation is different from the one in [1]. I would like people to compare the two approaches and decide which direction is better.

[1] https://postgr.es/m/[email-masked]

Best regards,

---

From: Chao Li <[email-masked]>
Date: Wed, 10 Jun 2026 17:12:58 +0800

> On Jun 10, 2026, at 17:05, Chao Li <[email-masked]> wrote:
> 
> 
> 

Oops! Forgot the attachment. Here comes it.

Best regards,

---

From: jian he <[email-masked]>
Date: Sat, 13 Jun 2026 10:42:46 +0800

> I am still sending an updated version of this patch because my implementation is different from the one in [1]. I would like people to compare the two approaches and decide which direction is bette…

I tried your patch before but abandoned it due to many regression test failures.

RememberAllDependentForRebuilding
{
            default:
                /*
                 * We don't expect any other sorts of objects to depend on a
                 * 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;
}

RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, 0, NULL) scans for
all dependencies on the relation (not just a specific column, since attnum=0).
This is much broader than a column-level scan.

The above DEFAULT branch in the SWITCH currently only exempts the relation's own
row type (TypeRelationId). But a table can have many other kinds of dependents
— indexes, triggers, foreign keys, views, etc. — and any of them that
hitting this
DEFAULT branch would incorrectly fire:

elog(ERROR, "unexpected object depending on column: %s", ...)

We need to check that every possible object type that can depend on a relation
and ensure none of them fall through to that `elog(ERROR, ...)`.

In RememberAllDependentForRebuilding, we have `if (subtype ==
AT_AlterColumnType)` guards against AT_SetExpression, — those branches
silently skip AT_SetExpression, which is good.
But the real risk is the DEFAULT branch, which lacks such a guard and
will error on anything unexpected.

---

From: Chao Li <[email-masked]>
Date: Sat, 13 Jun 2026 14:01:40 +0800

> On Jun 13, 2026, at 10:42, jian he <[email-masked]> wrote:
> 

Thanks for looking. But I wonder which tests I missed?

I ran “make check-world” locally and passed. The CI tests all passed on the CF: https://commitfest.postgresql.org/patch/6867/

Best regards,

---

From: jian he <[email-masked]>
Date: Mon, 15 Jun 2026 14:43:36 +0800

> I ran “make check-world” locally and passed. The CI tests all passed on the CF: https://commitfest.postgresql.org/patch/6867/

Apologies for the confusion.

I meant that I actually tried the same idea earlier, but abandoned it
after seeing many regression test failures.

Your v2 patch passes all the regression tests because of the special handling
added to the DEFAULT branch in RememberAllDependentForRebuilding. However, the
point of my previous message was that I am worried this special
handling below might
not be bulletproof (if foundObject.classId is not TypeRelationId, then
elog(ERROR) will be reached):

  /*
  * 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;
+

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 8 Jul 2026 20:20:43 +0200

On 10.06.26 11:12, Chao Li wrote:
>> fter reading the other implementation in [1], I realized that I had missed the partial-index case, so I added coverage for that.
>>
>> [1]https://postgr.es/m/ 
>> [email-masked]
>>
>> Best regards,
>> --
>> Chao Li (Evan)

This has been addressed via the patch in the other thread.