Thread

Re: [PATCH] Rebuild CHECK constraints after generated column SET EXPRESSION

jian he <jian.universality@gmail.com> — 2026-05-14T04:36:56Z

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/