Fix SET EXPRESSION for virtual columns with whole-row dependencies

On Tue, Jun 9, 2026 at 2:23 PM Chao Li <li.evan.chao@gmail.com> wrote:

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.

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

On Jun 9, 2026, at 15:23, jian he <jian.universality@gmail.com> wrote:

On Tue, Jun 9, 2026 at 2:23 PM Chao Li <li.evan.chao@gmail.com> wrote:

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.

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

Thanks for pointing out that. I will review 6755.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Jun 9, 2026, at 15:28, Chao Li <li.evan.chao@gmail.com> wrote:

On Jun 9, 2026, at 15:23, jian he <jian.universality@gmail.com> wrote:

On Tue, Jun 9, 2026 at 2:23 PM Chao Li <li.evan.chao@gmail.com> wrote:

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.

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

Thanks for pointing out that. I will review 6755.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

After reading the other implementation in [1]/messages/by-id/CAJTYsWXOkyeDVbzymWc9sKrq7Y_MUv6XJXN4H9GfsBOPd3NJ+w@mail.gmail.com, 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]/messages/by-id/CAJTYsWXOkyeDVbzymWc9sKrq7Y_MUv6XJXN4H9GfsBOPd3NJ+w@mail.gmail.com. I would like people to compare the two approaches and decide which direction is better.

[1]: /messages/by-id/CAJTYsWXOkyeDVbzymWc9sKrq7Y_MUv6XJXN4H9GfsBOPd3NJ+w@mail.gmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Jun 10, 2026, at 17:05, Chao Li <li.evan.chao@gmail.com> wrote:

On Jun 9, 2026, at 15:28, Chao Li <li.evan.chao@gmail.com> wrote:

On Jun 9, 2026, at 15:23, jian he <jian.universality@gmail.com> wrote:

On Tue, Jun 9, 2026 at 2:23 PM Chao Li <li.evan.chao@gmail.com> wrote:

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.

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

Thanks for pointing out that. I will review 6755.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

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] /messages/by-id/CAJTYsWXOkyeDVbzymWc9sKrq7Y_MUv6XJXN4H9GfsBOPd3NJ+w@mail.gmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Oops! Forgot the attachment. Here comes it.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Wed, Jun 10, 2026 at 5:06 PM Chao Li <li.evan.chao@gmail.com> 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.

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.

On Jun 13, 2026, at 10:42, jian he <jian.universality@gmail.com> wrote:

On Wed, Jun 10, 2026 at 5:06 PM Chao Li <li.evan.chao@gmail.com> 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.

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

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,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Sat, Jun 13, 2026 at 2:02 PM Chao Li <li.evan.chao@gmail.com> wrote:

On Jun 13, 2026, at 10:42, jian he <jian.universality@gmail.com> wrote:

On Wed, Jun 10, 2026 at 5:06 PM Chao Li <li.evan.chao@gmail.com> 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.

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

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/

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

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.

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/
CAJTYsWXOkyeDVbzymWc9sKrq7Y_MUv6XJXN4H9GfsBOPd3NJ+w@mail.gmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Oops! Forgot the attachment. Here comes it.

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