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

From: Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Rebuild CHECK constraints after generated column SET EXPRESSION
Date: 2026-05-15 04:00:08
Message-ID: CAJTYsWWw7s0N2hnAW9pfhkSx-aN=wELErrBEicoPMnmmS1e7kg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Fri, 15 May 2026 at 08:22, jian he <jian(dot)universality(at)gmail(dot)com> wrote:

> On Fri, May 15, 2026 at 1:06 AM Ayush Tiwari
> <ayushtiwari(dot)slg01(at)gmail(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message vellaipandiyan sm 2026-05-15 04:15:42 Review comments on createPartitions() in pgbench.c
Previous Message shveta malik 2026-05-15 03:58:35 Re: synchronized_standby_slots behavior inconsistent with quorum-based synchronous replication