public inbox for [email protected]  

help / color / mirror / Atom feed

From: Zsolt Parragi <[email protected]>
To: Chao Li <[email protected]>
Cc: Postgres hackers <[email protected]>
Subject: Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
Date: Mon, 26 Jan 2026 17:26:26 +0000
Message-ID: <CAN4CZFPX_t9q4W=jTh4jW4TOz=Xhfv0gPi9jG0MfXv9Ek7BO5w@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAEoWx2kggo1N2kDH6OSfXHL_5gKg3DqQ0PdNuL4LH4XSTKJ3-g@mail.gmail.com>
	<CAEoWx2k5800OjHO5KhTDv4JFYpDmyfh3MTtuB=7PP-0hLmG8yQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAN4CZFMUJWM0veLK93Ew8mYaL5pcU6G550STh6AZ-_LWV2zS+w@mail.gmail.com>
	<[email protected]>

+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.

view thread (11+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
  In-Reply-To: <CAN4CZFPX_t9q4W=jTh4jW4TOz=Xhfv0gPi9jG0MfXv9Ek7BO5w@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox