Re: tablecmds: reject CLUSTER ON for partitioned tables earlier

From: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
To: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
Date: 2026-01-23 07:43:07
Message-ID: CAN4CZFMUJWM0veLK93Ew8mYaL5pcU6G550STh6AZ-_LWV2zS+w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello!

A simple patch and generally looks good, I only have a few observations.

> “ALTER TABLE … CLUSTER ON” and "SET WITHOUT CLUSTER" are not supported for
> partitioned tables, but currently ATPrepCmd() allows them through and they
> only fail later at execution time.

Looking at the ALTER TABLE documentation, for other options there is a
mention like "This form is not currently supported on partitioned
tables." / "This form is not supported for partitioned tables."

I don't see this mentioned for CLUSTER or INHERIT. Maybe it would be
better to also mention this in the documentation?

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

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.

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.
*/

tablecmds.c:17860 - this check in ATExecDropInherit is now redundant,
since we already have it in ATPrepChangeInherit

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2026-01-23 07:53:06 Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
Previous Message Michael Paquier 2026-01-23 07:03:59 Some tests for TOAST, STORAGE MAIN/EXTENDED