+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 Otherwise the patches look good. 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. > 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.
