Hi,
On Fri, 15 May 2026 at 08:22, jian he <[email protected]> wrote: > On Fri, May 15, 2026 at 1:06 AM Ayush Tiwari > <[email protected]> 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
