| From: | Kirill Reshke <reshkekirill(at)gmail(dot)com> |
|---|---|
| To: | jian he <jian(dot)universality(at)gmail(dot)com> |
| Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter(at)eisentraut(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Fix ALTER TABLE DROP EXPRESSION with inheritance hierarchy |
| Date: | 2025-12-29 20:56:00 |
| Message-ID: | CALdSSPjeqcprfO4jmiUpmDqZmyMw0ZjN0De7jEuY_wFXXzYoag@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, 11 Nov 2025 at 11:43, jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> On Wed, Nov 5, 2025 at 2:31 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Masahiko Sawada | 2025-12-29 22:14:34 | Re: Assertion failure in SnapBuildInitialSnapshot() |
| Previous Message | Kirill Reshke | 2025-12-29 20:40:27 | Re: TAB completion for ALTER TABLE ... ALTER CONSTRAINT ... ENFORCED |