Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
Chao Li <li.evan.chao@gmail.com>
From: Chao Li <li.evan.chao@gmail.com>
To: Zsolt Parragi <zsolt.parragi@percona.com>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-01-26T23:13:04Z
Lists: pgsql-hackers
Attachments
- v6-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch (application/octet-stream) patch v6-0001
- v6-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch (application/octet-stream) patch v6-0002
> On Jan 27, 2026, at 01:26, Zsolt Parragi <zsolt.parragi@percona.com> wrote: > > +ALTER TABLE nonpartitioned INHERIT partitioned; -- ok > ERROR: cannot inherit from partitioned table "partitioned" > -- cannot add NO INHERIT constraint to partitioned tables > > That comment should be fail Fixed. > > Otherwise the patches look good. > Thanks a lot for confirming. > The rest is about the two checks that seem redundant to me - I don't > have a problem with leaving them as is, but they do seem redundant to > me. > >> So, I would leave the check there, maybe use a separate discussion for removal of the check. > > I tried to find a way to trigger it and couldn't figure out anything, > to me it seems unreachable. > >> 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. > > VACUUM FULL always passes InvalidOid to the cluster_rel for the index > parameter, so we can't hit the error. > > CLUSTER is more difficult to follow, but to me that also seems like to > never hit this error, and the behavior I see is also described in the > documentation (mark_index_clustered is only called for leaf > partitions, where it works). Following the calls in the code also > shows the same to me, that this method is now only called for > partitions. I will need more investigation on this, so let’s leave it for a seperate discussion. > >> No, the check is not redundant. It checks for child partitions, while ATPrepChangeInherit only blocks partitioned tables. > > And I have the same issue with this one: I modified that error in > ATExecDropInherit to an assertion locally. The test suite had no new > failures, I also tried to write a few tests manually, but I wasn't > able to trigger it. Maybe I'm missing something, but I think it's > redundant now. I added two new test cases in 0002 that trigger the check. BTW, this is the CF entry: https://commitfest.postgresql.org/patch/6415/. You may mark yourself as a reviewer, and once you consider the patch is ready to go, would you mind change the status to Ready For Committer? PFA v6. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Commits
-
Refactor some code around ALTER TABLE [NO] INHERIT
- 1a7ccd2b332d 19 (unreleased) landed
-
Add regression test for ALTER TABLE .. NO INHERIT on typed tables
- cbf9a72993ec 19 (unreleased) landed
-
Reject ALTER TABLE .. CLUSTER earlier for partitioned tables
- c336133c65a7 19 (unreleased) landed
-
Ignore partitioned indexes where appropriate
- 05fb5d661925 11.0 cited