| From: | Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Greg Sabino Mullane <htamfids(at)gmail(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: ALTER TABLE: warn when actions do not recurse to partitions |
| Date: | 2026-03-09 13:02:06 |
| Message-ID: | 14669a83-c7b4-4cdf-890c-dceecd025ee1@uni-muenster.de |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Chao
On 09/03/2026 07:46, Chao Li wrote:
> I agree with you that the NOTICE should only be emitted when the action succeeds.
Cool. v5 fixes this issue.
The error is now emitted prior to any NOTICE:
postgres=# ALTER TABLE m ALTER COLUMN a SET COMPRESSION pglz;
ERROR: column data type integer does not support compression
> Actually, there was another known issue in v4. Since an ALTER TABLE command may contain multiple sub-commands, the NOTICE and HINT could be repeated multiple times, and the HINT was completely duplicate.
Nice. The messages are now collected in CollectPartitionNoRecurseNotice
and emitted in EmitPartitionNoRecurseNotice, and duplicates are ignored,
if applicable ...
ALTER TABLE m
ALTER COLUMN b SET COMPRESSION pglz,
ALTER COLUMN b SET COMPRESSION pglz,
DISABLE RULE r,
ENABLE ROW LEVEL SECURITY,
FORCE ROW LEVEL SECURITY,
REPLICA IDENTITY FULL,
OWNER TO u1;
NOTICE: ALTER action ALTER COLUMN ... SET COMPRESSION on relation "m"
does not affect present partitions
NOTICE: ALTER action DISABLE RULE on relation "m" does not affect
present partitions
NOTICE: ALTER action ENABLE ROW SECURITY on relation "m" does not
affect present partitions
NOTICE: ALTER action FORCE ROW SECURITY on relation "m" does not affect
present partitions
NOTICE: ALTER action REPLICA IDENTITY on relation "m" does not affect
present partitions
NOTICE: ALTER action OWNER TO on relation "m" does not affect present
partitions
HINT: Partitions may be modified individually, or specify ONLY to
suppress this message.
ALTER TABLE
---
ALTER TABLE ONLY m
ALTER COLUMN b SET COMPRESSION pglz,
ALTER COLUMN b SET COMPRESSION pglz,
DISABLE RULE r,
ENABLE ROW LEVEL SECURITY,
FORCE ROW LEVEL SECURITY,
REPLICA IDENTITY FULL,
OWNER TO u1;
ALTER TABLE
... which brings me to a few nitpicks:
1) A test containing multiple sub-commands in an ALTER TABLE (as shown
above) would be nice.
2) There are some tests with misleading comments. For instance:
-- set column attribute on partitioned table should get a notice
ALTER TABLE list_parted4 ALTER COLUMN b SET (n_distinct = 0.2);
ALTER TABLE list_parted4 ALTER COLUMN b RESET (n_distinct);
ALTER TABLE ONLY list_parted4 ALTER COLUMN b SET (n_distinct = 0.2);
ALTER TABLE ONLY list_parted4 ALTER COLUMN b RESET (n_distinct);
The last two will not emit any NOTICE, since they're using the keyword
ONLY. I'd say that either these ONLY tests need to be in a different
code block with a proper comment, or the current comments need to be
changed to make it clear, e.g. "set column attribute on partitioned
table should get a NOTICE; ONLY suppresses the NOTICE"
I also reviewed the code and it LGTM!
Thanks for the patch!
Best, Jim
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ranier Vilela | 2026-03-09 13:07:19 | Simplifies checks (src/bin/pg_dump/pg_backup_archiver.c) |
| Previous Message | Ranier Vilela | 2026-03-09 12:54:20 | Re: Avoid resource leak (src/bin/pg_dump/pg_dumpall.c) |