Re: Fix ALTER TABLE DROP EXPRESSION with inheritance hierarchy

From: Kirill Reshke <reshkekirill(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix ALTER TABLE DROP EXPRESSION with inheritance hierarchy
Date: 2025-08-25 13:04:25
Message-ID: CALdSSPieqqBvgex+OFKwyEMZx-P9sc-D9MGpBbCSQ8OhLmYn_w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi!

On Sun, 24 Aug 2025 at 14:05, jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> hi.
>
> --this ALTER COLUMN DROP EXPRESSION work as expected
> DROP TABLE IF EXISTS parent cascade;
> CREATE TABLE parent (a int, d INT GENERATED ALWAYS AS (11) STORED);
> CREATE TABLE child () INHERITS (parent);
> ALTER TABLE parent ALTER COLUMN d DROP EXPRESSION;
>
>
> ----- the below (ALTER COLUMN DROP EXPRESSION) should also work.
> -----
> DROP TABLE IF EXISTS parent cascade;
> CREATE TABLE parent (a int, d INT GENERATED ALWAYS AS (11) STORED);
> CREATE TABLE child () INHERITS (parent);
> CREATE TABLE grandchild () INHERITS (child);
> ALTER TABLE parent ALTER COLUMN d DROP EXPRESSION;
>
> but currently it will generated error:
> ERROR: 0A000: ALTER TABLE / DROP EXPRESSION must be applied to child tables too
> LOCATION: ATPrepDropExpression, tablecmds.c:8734
>
> The attached patch fixes this potential issue.

Good catch, I agree that current behaviour is not correct.

However, I am not terribly sure that your suggested modification is
addressing the issues appropriately.

My understanding is that this if statement protects when user
specifies ONLY option in ALTER TABLE:

> if (!recurse &&
> - find_inheritance_children(RelationGetRelid(rel), lockmode))
> + find_inheritance_children(RelationGetRelid(rel), lockmode) &&
> + RelationGetRelid(rel) == context->relid)

So we need to detect if the user did ALTER TABLE or ALTER TABLE ONLY.
And we have two parameters passed to ATPrepDropExpression: "recurse"
and "recursing".
First is about whether the user specified ONLY option and second is
about if we are recursing in our AT code. So maybe fix it as in
attached?

===

I also spotted potential enhancement in the error message: we can add
HINT here, akin to partitioned table processing. WHYT?

```
reshke=# begin;
BEGIN
reshke=*# ALTER TABLE parent ALTER COLUMN d DROP EXPRESSION;
ALTER TABLE
reshke=*# rollback ;
ROLLBACK
reshke=# begin;
BEGIN
reshke=*# ALTER TABLE ONLY parent ALTER COLUMN d DROP EXPRESSION;
ERROR: ALTER TABLE / DROP EXPRESSION must be applied to child tables too
HINT: Do not specify the ONLY keyword.
reshke=!# rollback ;
ROLLBACK
```

--
Best regards,
Kirill Reshke

Attachment Content-Type Size
v2-0001-Fix-ALTER-TABLE-DROP-EXPRESSION-with-inheritance-.patch application/octet-stream 8.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Antonin Houska 2025-08-25 13:09:18 Re: Adding REPACK [concurrently]
Previous Message Shlok Kyal 2025-08-25 13:02:41 Re: POC: enable logical decoding when wal_level = 'replica' without a server restart