> On Jan 23, 2026, at 15:43, Zsolt Parragi <zsolt.parragi@percona.com> wrote:
>
> Also, there seems to be no test for partitioned NO INHERIT, since the
> patch changes it, it could also add a test case to verify the
> behavior.
>
> rg "INHERIT" | grep "cannot be performed"
> src/test/regress/expected/alter_table.out:ERROR: ALTER action INHERIT
> cannot be performed on relation "partitioned"
>
> rg "NO INHERIT" | grep "cannot be performed"
> # no result
Added a test case.
>
> tablecmds.c:5202
> case AT_DropInherit: /* NO INHERIT */
> ATSimplePermissions(cmd->subtype, rel,
> - ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_FOREIGN_TABLE);
> + ATT_TABLE | ATT_FOREIGN_TABLE);
> /* This command never recurses */
> + ATPrepChangeInherit(rel);
> /* No command-specific prep needed */
>
> That last comment seems to be a leftover, it's no longer true with this change.
Deleted the comment.
>
> tablecmds.c:17289 trailing whitespace (in the empty line)
> /*
> + * ALTER TABLE INHERIT
> + *
> + * Add a parent to the child's parents. This verifies that all the columns and
> + * check constraints of the parent appear in the child and that they have the
> + * same data types and expressions.
> + *
> * Return the address of the new parent relation.
> */
>
Deleted the whitespace.
> tablecmds.c:17860 - this check in ATExecDropInherit is now redundant,
> since we already have it in ATPrepChangeInherit
No, the check is not redundant. It checks for child partitions, while ATPrepChangeInherit only blocks partitioned
tables.
>
>> Before the patch:
>> ```
>> evantest=# ALTER TABLE p_test CLUSTER ON idx_p_test_id;
>> ERROR: cannot mark index clustered in partitioned table
>
> Can we still reach the original error in mark_index_clustered somehow?
> I don't see any examples in the test suite, or execution paths when I
> have looked at the code, and it can be confusing to see a different
> error code/message there.
The portioned table check was added to mark_index_clustered with 05fb5d6. In the commit, the test case "ALTER TABLE
clstrpartCLUSTER ON clstrpart_idx;” was added as well, so my best guess the check is now no longer needed. I tried to
removethe check, and “make check” still passes.
However, there is a call path: vacuum -> vacuum_rel -> cluster_rel -> rebuild_relation -> mark_index_clustered. I am
notsure if the check plays some role there.
So, I would leave the check there, maybe use a separate discussion for removal of the check.
PFA v5.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
