Thread

Re: Fix ALTER TABLE DROP EXPRESSION with inheritance hierarchy

Kirill Reshke <reshkekirill@gmail.com> — 2025-12-29T20:56:00Z

On Tue, 11 Nov 2025 at 11:43, jian he <jian.universality@gmail.com> wrote:
>
> On Wed, Nov 5, 2025 at 2:31 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> >
> > If it actually matters for DROP EXPRESSION, then the answer is
> > probably "we can't use ATSimpleRecursion for DROP EXPRESSION".
> > ATSimpleRecursion is meant for cases where each table can be
> > processed independently, regardless of its position in the
> > hierarchy.
> >
>
> ATPrepAlterColumnType will call ATPrepCmd, which will call again
> ATPrepAlterColumnType.
> Similarly, we can remove ATSimpleRecursion, and let
> ATPrepDropExpression call ATPrepCmd
> but that will just be the duplication of ATSimpleRecursion, i think.
>
>     /*
>      * Recurse manually by queueing a new command for each child, if
>      * necessary. We cannot apply ATSimpleRecursion here because we need to
>      * remap attribute numbers in the USING expression, if any.
> ATPrepAlterColumnType has the above comments.
> but here, we don't need to do anything between "rel" and "parent_rel".
>
> ATPrepDropExpression logic is quite simple, it only needs to check the
> a. the original source relation is species ONLY or not.
> b. the original source relation is inherited column or not.
>
> ALTER TABLE ONLY parent ALTER COLUMN d DROP EXPRESSION;
> it will skip ATSimpleRecursion, because first time recurse is
> false.(keyword ONLY specified)
> the first time enter ATPrepDropExpression, both "recurse" and
> "recursing" is false.
>
>     if (!recurse && !recursing &&
>         find_inheritance_children(RelationGetRelid(rel), lockmode))
>         ereport(ERROR,
>                 errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
>                 errmsg("ALTER TABLE / DROP EXPRESSION must be applied
> to child tables too"),
>                 errhint("Do not specify the ONLY keyword."));
>
> so the above code makes sense to me.
> Because we only need to do this check once.
>
> Summary:
> 1. We need ATSimpleRecursion so that AlteredTableInfo structures for child
> relations are populated too, so generated expressions are dropped from all
> inherited tables.
>
> 2. ATPrepDropExpression only checks the original table specified in the ALTER
> TABLE command, and does not apply the check to its child relations.
> (for example: ALTER TABLE parent ALTER COLUMN d DROP EXPRESSION;
> ATPrepDropExpression check only applies to "parent" not its child relation).


Hi!
I did take another look at this thread. I agree this "recurse and
recursing" logic is a little confusing.
Anyway, are you saying that v3 from this thread is a fix you are OK with?


-- 
Best regards,
Kirill Reshke