Re: tablecmds: reject CLUSTER ON for partitioned tables earlier

> On Jan 23, 2026, at 15:43, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)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 clstrpart CLUSTER ON clstrpart_idx;” was added as well, so my best guess the check is now no longer needed. I tried to remove the check, and “make check” still passes.

However, there is a call path: vacuum -> vacuum_rel -> cluster_rel -> rebuild_relation -> mark_index_clustered. I am not sure 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/