> On Mar 9, 2026, at 21:02, Jim Jones <[email protected]> wrote: > > 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.
Added a test case that contains multiple sub-commands. > 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 updated the comments like: ``` -- enable/disable rules on partitioned tables without ONLY should get a notice ``` “Without ONLY” should eliminate the confusion. > I also reviewed the code and it LGTM! > Thanks a lot for your test and review. PFA v6, addressed Jim’s comments as explained above. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
v6-0001-ALTER-TABLE-emit-NOTICE-when-ONLY-is-omitted-but-.patch
Description: Binary data
