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

On Wed, May 13, 2026 at 6:53 PM Ayush Tiwari
<ayushtiwari(dot)slg01(at)gmail(dot)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/