Re: ALTER TABLE: warn when actions do not recurse to partitions

> On Mar 10, 2026, at 23:32, Greg Sabino Mullane <htamfids(at)gmail(dot)com> wrote:
>
> Review of v6:

Thank you very much for the review.

>
> typedef struct partitionNoRecurseNotice
> {
> List *notices;
> } partitionNoRecurseNotice;
> Not sure why we need a struct here, rather than just passing the list around?

Initially I thought there might be a few fields in the structure, but ended up only one List field. Yes, this structure is not needed now. Removed it in v7.

>
> Also should be PartitionNoRecurseNotice (CamelCase)
>
> foreach(cell, postNotice->notices)
> {
> if (strcmp((char *) lfirst(cell), notice_msg) == 0)
> {
> pfree(notice_msg);
> found = true;
> break;
> }
> }
>
> This seems a lot of extra work that could be avoided. Since we know each message is unique to the cmdtype/AlterTableType and the rel/Relation combination, use those two to drive the duplicate check. Then we can only build the notice_msg if needed! Perhaps adding two more fields to that lonely struct above?

Are you concerning that rendering the full message text is the extra work? This is not a hot path, so I don’t think that would be a big deal. Actually, adding two more fields sounds more expensive.

>
> partitionNoRecurseNotice * postNotice);
>
> postNotice is a little misleading - maybe pending_notices or just notices?

Yes, “pending” makes sense. Updated in v7.

>
> does not affect present partitions
>
> s/present/existing/g
> CollectPartitionNoRecurseNotice(AT_SetSchema, rel, stmt->relation->inh, false, &postNotice);
>
> This hard-coded AT_SetSchema just to return a "SET SCHEMA" later on feels hacky. Don't have a workaround off the top of my head, just registering my mild unease. :)

Yes, as SET SCHEMA doesn’t go through the standard ALTER TABLE process: AlterTable() -> ATController() -> ATPrepCmd().

If you get some idea, please let me know.

>
> /* Emit a notice only if there are partitions */
> if (nparts == 0)
> return;
>
> It doesn't look like this particular case is tested. Other than that, the tests look very good.

Good catch. Added a test case to cover that.

PFA v7: addressed Greg’s review comments.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/