[PATCH] Rebuild CHECK constraints after generated column SET EXPRESSION

Hi.

In case you are wondering, I already handled the whole-row cases for
ALTER TABLE DROP COLUMN and ALTER COLUMN SET DATA TYPE in
https://commitfest.postgresql.org/patch/5988 and
https://commitfest.postgresql.org/patch/6055

However, I missed the ALTER COLUMN SET EXPRESSION scenario.

RememberAllDependentForRebuilding with (attnum = 0) is essentially
asking any objects depend on this relation,
It will certainly catch many whole-row referenced dependent objects,
however, I wouldn’t be surprised if unintended corner cases exist.

CREATE TABLE r2 (a int, b int GENERATED ALWAYS AS (a * 10) STORED);
ALTER TABLE r2 ADD CONSTRAINT cc CHECK (a IS NOT NULL);
CREATE INDEX r2_idx ON r2 (a);
CREATE POLICY p3 ON r2 AS PERMISSIVE USING (a IS NOT NULL);
select classid::regclass, * from pg_depend where refobjid =
'r2'::regclass::oid and classid in ('pg_policy'::regclass);

The examples above show that RLS policies can have two dependencies on the
relation: one on the specific column, and another on the relation itself.
``RememberAllDependentForRebuilding with (attnum = 0)`` cannot cope with this.

ATRewriteTables->finish_heap_swap->reindex_relation may reindex the
relation, but
AlteredTableInfo->changedIndexOids should still remember the whole-row
Var references index objects.

For ALTER COLUMN SET EXPRESSION, no need to worry about whole-row
referenced triggers.BEFORE trigger referencing the whole-row
(including generated column) is not allowed (see
CreateTriggerFiringOn: ```if (!whenClause &&stmt->whenClause)```), and
ALTER COLUMN SET EXPRESSION will work fine with AFTER
triggersthat have whole-row reference.

The attached v2 includes support for ALTER COLUMN SET EXPRESSION on columns
referenced by whole-row indexes, check constraints, and RLS policies.

The code pattern is more or less simialr to
https://commitfest.postgresql.org/patch/6055.
I should rebase https://commitfest.postgresql.org/patch/6055 BTW.

--
jian
https://www.enterprisedb.com/

Hi,

On Wed, 13 May 2026 at 10:50, jian he <jian.universality@gmail.com> wrote:

Hi.

In case you are wondering, I already handled the whole-row cases for
ALTER TABLE DROP COLUMN and ALTER COLUMN SET DATA TYPE in
https://commitfest.postgresql.org/patch/5988 and
https://commitfest.postgresql.org/patch/6055

However, I missed the ALTER COLUMN SET EXPRESSION scenario.

RememberAllDependentForRebuilding with (attnum = 0) is essentially
asking any objects depend on this relation,
It will certainly catch many whole-row referenced dependent objects,
however, I wouldn’t be surprised if unintended corner cases exist.

CREATE TABLE r2 (a int, b int GENERATED ALWAYS AS (a * 10) STORED);
ALTER TABLE r2 ADD CONSTRAINT cc CHECK (a IS NOT NULL);
CREATE INDEX r2_idx ON r2 (a);
CREATE POLICY p3 ON r2 AS PERMISSIVE USING (a IS NOT NULL);
select classid::regclass, * from pg_depend where refobjid =
'r2'::regclass::oid and classid in ('pg_policy'::regclass);

The examples above show that RLS policies can have two dependencies on the
relation: one on the specific column, and another on the relation itself.
``RememberAllDependentForRebuilding with (attnum = 0)`` cannot cope with
this.

ATRewriteTables->finish_heap_swap->reindex_relation may reindex the
relation, but
AlteredTableInfo->changedIndexOids should still remember the whole-row
Var references index objects.

For ALTER COLUMN SET EXPRESSION, no need to worry about whole-row
referenced triggers.BEFORE trigger referencing the whole-row
(including generated column) is not allowed (see
CreateTriggerFiringOn: ```if (!whenClause &&stmt->whenClause)```), and
ALTER COLUMN SET EXPRESSION will work fine with AFTER
triggersthat have whole-row reference.

The attached v2 includes support for ALTER COLUMN SET EXPRESSION on columns
referenced by whole-row indexes, check constraints, and RLS policies.

Thanks for picking this up, and for going much wider than I did
(indexes + policies, plus the per-expression check rather than my
"rebuild all CHECK constraints" hammer).

Patch overall looks good to me, have few small comments.

1. Typos and grammar nits

* generated_stored.sql / generated_virtual.sql (and the matching
.out files):
"-- indedx with whole-row reference need rebuild"
-> "-- index with whole-row reference needs rebuild"
"AFFTER TRIGGER" -> "AFTER TRIGGER"

* tablecmds.c: GeRelAssociatedPolicies()
The function name is missing the 't' -- should be
GetRelAssociatedPolicies() to match the rest of the file.

* var.c: "Use exprContainWholeRow to check whole-row var existsence
when in doubt." -> "existence".

* exprContainWholeRow: I think PostgreSQL naming style favours
under_score_lowercase for new functions (compare pull_varattnos,
contain_var_clause). expr_contains_wholerow_var (or similar)
would fit better?

2. The error message text:

errmsg("cannot alter generation expression of a column used in a
policy definition"),
errdetail("%s depends on column \"%s\"", ..., colName)

The DETAIL phrasing is slightly misleading. In the whole-row case
the policy doesn't depend on the column directly; it depends on the
relation, and the column happens to be part of the row value the
policy reads. Maybe:

errmsg("cannot alter generation expression of column \"%s\" of
relation \"%s\"",
colName, RelationGetRelationName(rel)),
errdetail("%s references the whole row.",
getObjectDescription(&pol_obj, false))

or similar. Saying "the whole row" in the DETAIL also gives users a
hint about how to fix it (e.g. rewrite the policy to reference
specific columns).

3. Minor: the `wholerow_idxoids` local in
RememberWholeRowDependentForRebuilding() is declared and used only
via `list_member_oid(...)`, but is never appended to anywhere in
this function. As written it's dead code?

4. Locking: GetRelAssociatedPolicies() opens pg_depend with
RowExclusiveLock, but it only reads. I think AccessShareLock should be
enough, matching the other lookup helpers in this file.

5. nit: Cosmetic: the same pull_varattnos + bms_is_member +
bms_free(expr_attrs) + reset-to-NULL pattern is repeated three
times (CHECK constraint, indexprs, indpred). A small helper
`expr_has_wholerow_var(Node *expr)` would make the function much
shorter and easier to read.

Thoughts?

Regards,
Ayush

Hello!

+ALTER TABLE gtest20c ALTER COLUMN b SET EXPRESSION AS (NULL::int);  -- violates constraint
+ERROR:  check constraint "whole_row_check" of relation "gtest20c" is violated by some row
+-- indedx with whole-row reference need rebuild
+CREATE TABLE gtest20d (a int, b int GENERATED ALWAYS AS (a * 2) STORED);
+INSERT INTO gtest20d VALUES (1), (1);

This is in generated_virtual.sql, is that STORED a copy-paste mistake?

On Wed, May 13, 2026 at 6:53 PM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

Patch overall looks good to me, have few small comments.

2. The error message text:

errmsg("cannot alter generation expression of a column used in a policy definition"),
errdetail("%s depends on column \"%s\"", ..., colName)

The DETAIL phrasing is slightly misleading. In the whole-row case
the policy doesn't depend on the column directly; it depends on the
relation, and the column happens to be part of the row value the
policy reads. Maybe:

errmsg("cannot alter generation expression of column \"%s\" of relation \"%s\"",
colName, RelationGetRelationName(rel)),
errdetail("%s references the whole row.",
getObjectDescription(&pol_obj, false))

or similar. Saying "the whole row" in the DETAIL also gives users a
hint about how to fix it (e.g. rewrite the policy to reference
specific columns).

I changed it to:
+                    if (subtype == AT_SetExpression)
+                        ereport(ERROR,
+                                errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                errmsg("ALTER TABLE / SET EXPRESSION
is not supported for generated columns in tables that are part of a
policy definition"),
+                                errdetail("%s contains whole row
references.", getObjectDescription(&pol_obj, false)));

5. nit: Cosmetic: the same pull_varattnos + bms_is_member +
bms_free(expr_attrs) + reset-to-NULL pattern is repeated three
times (CHECK constraint, indexprs, indpred). A small helper
`expr_has_wholerow_var(Node *expr)` would make the function much
shorter and easier to read.

RememberWholeRowDependentForRebuilding handles CHECK constraint,
indexprs, indpred, and policy; we should expect it to be big.
expr_has_wholerow_var won't help it become more readable, IMHO.

All your other points are being addressed.
Zsolt Parragi mentioned copy-paste mistake has been corrected.
And other minor cosmetic changes.

--
jian
https://www.enterprisedb.com/

Hi,

On Thu, 14 May 2026 at 10:07, jian he <jian.universality@gmail.com> wrote:

RememberWholeRowDependentForRebuilding handles CHECK constraint,
indexprs, indpred, and policy; we should expect it to be big.
expr_has_wholerow_var won't help it become more readable, IMHO.

All your other points are being addressed.
Zsolt Parragi mentioned copy-paste mistake has been corrected.
And other minor cosmetic changes.

Thanks for the v3. I checked it applied cleanly and test passes.

I've two minor follow-up comments/questions:

1. The "-- indedx with whole-row reference need rebuild" comment is
still in the new SQL/expected blocks for both generated_stored and
generated_virtual:
"-- index with whole-row reference needs rebuild"

2. The new policy error message:

errmsg("ALTER TABLE / SET EXPRESSION is not supported for
generated columns in tables that are part of a policy
definition"),
errdetail("%s contains whole row references.", ...)

I still find this wording a bit awkward. Quoting "ALTER TABLE /
SET EXPRESSION" as a syntactic form in the message is unusual for
tablecmds.c, and the sentence is long. Could we keep it closer to
the surrounding style.

Just flagging it because this is user-facing text.

Regards,
Ayush

On Fri, May 15, 2026 at 1:06 AM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

I've two minor follow-up comments/questions:

1. The "-- indedx with whole-row reference need rebuild" comment is
still in the new SQL/expected blocks for both generated_stored and
generated_virtual:
"-- index with whole-row reference needs rebuild"

Thanks for finding the typo!

I did another round of code cleanup and cosmetic refactoring. Main
idea still the same: loop through
pg_constraint, pg_index, and pg_policy to locate all objects
containing whole-row
references, then error reporting or remember them for recreation.

The commit message was also updated.
Later, I will add this thread to
https://wiki.postgresql.org/wiki/PostgreSQL_19_Open_Items

2. The new policy error message:

errmsg("ALTER TABLE / SET EXPRESSION is not supported for
generated columns in tables that are part of a policy
definition"),
errdetail("%s contains whole row references.", ...)

I still find this wording a bit awkward. Quoting "ALTER TABLE /
SET EXPRESSION" as a syntactic form in the message is unusual for
tablecmds.c, and the sentence is long. Could we keep it closer to
the surrounding style.

+                    if (subtype == AT_SetExpression)
+                        ereport(ERROR,
+                                errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                errmsg("cannot alter generation
expression of table %s because %s uses its row type",
+                                       RelationGetRelationName(rel),
+                                       getObjectDescription(&pol_obj, false)),
+                                errdetail("You might need to drop %s
first.", getObjectDescription(&pol_obj, false)));
What do you think?

--
jian
https://www.enterprisedb.com/

Hi,

On Fri, 15 May 2026 at 08:22, jian he <jian.universality@gmail.com> wrote:

On Fri, May 15, 2026 at 1:06 AM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

The commit message was also updated.
Later, I will add this thread to
https://wiki.postgresql.org/wiki/PostgreSQL_19_Open_Items

Sounds good, please do.

+                    if (subtype == AT_SetExpression)
+                        ereport(ERROR,
+                                errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                errmsg("cannot alter generation
expression of table %s because %s uses its row type",
+                                       RelationGetRelationName(rel),
+                                       getObjectDescription(&pol_obj,
false)),
+                                errdetail("You might need to drop %s
first.", getObjectDescription(&pol_obj, false)));
What do you think?

Thanks for v4.

The CHECK constraint and index handling looks good to me. Those are
the cases that need action after SET EXPRESSION: CHECK constraints need
to be revalidated, and whole-row expression/predicate indexes need to
be rebuilt.

One question about the policy part: do we need to disallow SET
EXPRESSION for whole-row policy references at all?

For ordinary column references, RememberAllDependentForRebuilding()
already sees PolicyRelationId, but it only errors for
AT_AlterColumnType, not AT_SetExpression. That seems right to me:
changing a generated column's expression doesn't change the row type or
require rewriting/recreating the policy; the policy expression is
evaluated at query time. A whole-row reference seems similar, except
that pg_depend records it as a relation-level dependency.

The new behavior also blocks cases like altering r2 because a policy on
r1 mentions r2's row type. That feels overly conservative to me,
because SET EXPRESSION on r2 does not rebuild/recheck policy p3 on r1.

On the error message itself: if the policy path stays, I think the
cross-table case needs to be clearer. For example, "cannot alter
generation expression of table r2 because policy p3 on table r1 uses
its row type" leaves "its" a bit ambiguous. Maybe the DETAIL could
say that the policy contains a whole-row reference to the table being
altered, so the user can see why a policy on another table is involved.

If there is a reason whole-row policies need special treatment for SET
EXPRESSION that direct column policy references don't, it'd be good to
spell that out in the comment or commit message. Otherwise, maybe the
policy scan/error can be dropped from this patch, leaving the CHECK and
index rebuild pieces.

Two small cleanup nits if the policy path stays:

1. `attnum` and `colName` are no longer referenced in
RememberWholeRowDependentForRebuilding(), so they can be dropped
from the signature.

2. The function asserts `subtype == AT_SetExpression`, but the policy
error sites still check `if (subtype == AT_SetExpression)`. One of
those seems redundant.

Regards,
Ayush

On Fri, May 15, 2026 at 12:00 PM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

One question about the policy part: do we need to disallow SET
EXPRESSION for whole-row policy references at all?

For ordinary column references, RememberAllDependentForRebuilding()
already sees PolicyRelationId, but it only errors for
AT_AlterColumnType, not AT_SetExpression.

For AT_SetExpression:
RememberAllDependentForRebuilding does not handle policy objects.
We can safely ignore policy objects that contain whole-row variable
references too.

Two small cleanup nits if the policy path stays:

1. `attnum` and `colName` are no longer referenced in
RememberWholeRowDependentForRebuilding(), so they can be dropped
from the signature.

In case we later need to cope with an ALTER TABLE command, such as
ALTER TABLE DROP COLUMN
and ALTER COLUMN SET DATA TYPE.
The signature also aligns with RememberAllDependentForRebuilding.

--
jian
https://www.enterprisedb.com/

Hi,

On Sat, 16 May 2026 at 11:38, jian he <jian.universality@gmail.com> wrote:

On Fri, May 15, 2026 at 12:00 PM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

One question about the policy part: do we need to disallow SET
EXPRESSION for whole-row policy references at all?

For ordinary column references, RememberAllDependentForRebuilding()
already sees PolicyRelationId, but it only errors for
AT_AlterColumnType, not AT_SetExpression.

For AT_SetExpression:
RememberAllDependentForRebuilding does not handle policy objects.
We can safely ignore policy objects that contain whole-row variable
references too.

Two small cleanup nits if the policy path stays:

1. `attnum` and `colName` are no longer referenced in
RememberWholeRowDependentForRebuilding(), so they can be dropped
from the signature.

In case we later need to cope with an ALTER TABLE command, such as
ALTER TABLE DROP COLUMN
and ALTER COLUMN SET DATA TYPE.
The signature also aligns with RememberAllDependentForRebuilding.

Thanks, v5 addresses my concern about policies. Ignoring whole-row
policy references for SET EXPRESSION makes sense to me, and the comment
explains the distinction clearly.

No further comments from me.

Regards,
Ayush

Hi
I tested v5-0001-recreate-wholerow-dependent-while-ALTER-COLUMN-SET-EXPRESSION.patch
on the current master.The patch applied cleanly and PostgreSQL built
successfully.
I was able to reproduce the issue on unpatched master using a
whole-row CHECK constraint referencing the generated column
through:CHECK ((gtest).b IS NOT NULL).
Without the patch,ALTER COLUMN SET EXPRESSION succeeded even though
existing rows no longer satisfied the constraint.
After applying the patch ,the same ALTER TABLE command correctly
failed with : ERROR:check constraint "whole_row_check" of relation
"gtest" is violated by some row.
The fix and regression coverage look good to me.

Regards,
Solai