| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> |
| 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-01-23 00:11:35 |
| Message-ID: | 8ECD9403-F0BB-4971-94CF-2709EEB4E3B9@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Jan 23, 2026, at 03:27, Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> wrote:
>
> Hi Chao
>
> On 22/01/2026 06:45, Chao Li wrote:
>> evantest=# alter table p_test replica identity full, alter column
>> username set (n_distinct = 0.1);
>> NOTICE: ALTER action REPLICA IDENTITY on relation "p_test" does not
>> affect present partitions
>> HINT: partitions may be modified individually, or specify ONLY to
>> suppress this message
>> NOTICE: ALTER action ALTER COLUMN ... SET on relation "p_test" does not
>> affect present partitions
>> HINT: partitions may be modified individually, or specify ONLY to
>> suppress this message
>> ALTER TABLE
>
>
> One could argue that encapsulating all conditions in
> EmitPartitionNoRecurseNotice(), meaning it is called all the time, is
> slightly inefficient, but the impact is really negligible in this case -
> and it is how it is done in similar functions in tablecmds.c :) The code
> LGTM.
Hi Jim, thanks a lot for the review.
>
> One small thing:
>
> errhint is supposed to be capitalised - see Error Message Style Guide[1]
Thanks for the info, I wasn’t aware of that. When I wrote the code, I searched “errhint” over the source tree, and didn’t find a standard to follow.
>
> "Detail and hint messages: Use complete sentences, and end each with a
> period. Capitalize the first word of sentences. Put two spaces after the
> period if another sentence follows (for English text; might be
> inappropriate in other languages)."
>
> ereport(NOTICE,
> errmsg("ALTER action %s on relation \"%s\" does not affect present
> partitions",
> action_str,
> RelationGetRelationName(rel)),
> errhint("partitions may be modified individually, or specify ONLY to
> suppress this message"));
>
> What about this?
>
> HINT: To update partitions, apply the command to each one individually,
> or specify ONLY to suppress this message.
Looks good. I will integrate your edit to the next version.
>
> I'll test the newly covered subcomands tomorrow.
Thanks again for testing. I will wait to see the test results and address all issues together in next version.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Chao Li | 2026-01-23 00:54:33 | Re: tablecmds: reject CLUSTER ON for partitioned tables earlier |
| Previous Message | Tom Lane | 2026-01-22 23:43:54 | Re: Assert when executing query on partitioned table |