> On Jan 23, 2026, at 15:43, Zsolt Parragi <[email protected]> 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/
v5-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch
Description: Binary data
v5-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch
Description: Binary data
