From: Chao Li <li.evan.chao@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_0C148671-6739-4850-B3C9-3A08AEBF678A"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.300.41.1.7\))
Subject: Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
Date: Fri, 23 Jan 2026 07:30:55 +0800
References: <CAEoWx2kggo1N2kDH6OSfXHL_5gKg3DqQ0PdNuL4LH4XSTKJ3-g@mail.gmail.com>
 <CAEoWx2k5800OjHO5KhTDv4JFYpDmyfh3MTtuB=7PP-0hLmG8yQ@mail.gmail.com>
To: Postgres hackers <pgsql-hackers@lists.postgresql.org>
In-Reply-To: <CAEoWx2k5800OjHO5KhTDv4JFYpDmyfh3MTtuB=7PP-0hLmG8yQ@mail.gmail.com>
Message-Id: <07773235-2E94-478F-BEF6-38C73B0553B8@gmail.com>
Archived-At: <https://www.postgresql.org/message-id/07773235-2E94-478F-BEF6-38C73B0553B8%40gmail.com>
Precedence: bulk


--Apple-Mail=_0C148671-6739-4850-B3C9-3A08AEBF678A
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> On Jan 22, 2026, at 17:01, Chao Li <li.evan.chao@gmail.com> wrote:
>=20
>=20
>=20
>> On Jan 21, 2026, at 11:55, Chao Li <li.evan.chao@gmail.com> wrote:
>>=20
>> Hi Hacker,
>>=20
>> I noticed this while working other patches related to =E2=80=9CALTER =
TABLE=E2=80=9D.
>>=20
>> =E2=80=9CALTER TABLE =E2=80=A6 CLUSTER ON=E2=80=9D and "SET WITHOUT =
CLUSTER" are not supported for partitioned tables, but currently =
ATPrepCmd() allows them through and they only fail later at execution =
time.
>>=20
>> This patch rejects these commands earlier by using the existing =
ATSimplePermissions() infrastructure in ATPrepCmd(), matching the =
handling of other unsupported ALTER TABLE actions on partitioned tables =
(such as SET LOGGED / SET UNLOGGED). This makes the behavior more =
consistent and simplifies the code path.
>>=20
>> As a result, the error reported for partitioned tables changes:
>>=20
>> Before the patch:
>> ```
>> evantest=3D# ALTER TABLE p_test CLUSTER ON idx_p_test_id;
>> ERROR:  cannot mark index clustered in partitioned table
>> ```
>>=20
>> With the patch:
>> ```
>> evantest=3D# ALTER TABLE p_test CLUSTER ON idx_p_test_id;
>> ERROR:  ALTER action CLUSTER ON cannot be performed on relation =
"p_test"
>> DETAIL:  This operation is not supported for partitioned tables.
>> ```
>>=20
>> Best regards,
>> --
>> Chao Li (Evan)
>> HighGo Software Co., Ltd.
>> https://www.highgo.com/
>>=20
>>=20
>>=20
>>=20
>> <v1-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch>
>=20
>=20
>=20
> Applying the same change to INHERIT/NO INHeRIT in v2-0002. Other than =
that, fixing 2 more things for INHERIT/NO INHERIT:
>=20
> * The header comment of ATPrepAddInherit() was a copy-paste mistake, =
it described something totally unrelated.
> * NO INHERIT didn=E2=80=99t call ATPrepAddInherit() to check early, so =
it had to go deeper and run unnecessary checks.
>=20
> Basically, 0001 and 0002 do the same thing on two sub-commands. If =
accepted, they can be squashed.
>=20
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>=20
> =
<v2-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch><v2-0002-=
tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch>

PFA v3:

0001 is the same as v2. In 0002:

* Restored the header comment of ATPrepAddInherit, because I realized =
the should belong to ATExecAddInherit.
* Renamed ATPrepAddInherit to ATPrepChangeInherit.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/


--Apple-Mail=_0C148671-6739-4850-B3C9-3A08AEBF678A
Content-Disposition: attachment;
	filename=v3-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v3-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=2075a3d94296e83ec5cc82b98f606227503f2c575e=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20"Chao=20Li=20(Evan)"=20<lic@highgo.com>=0A=
Date:=20Wed,=2021=20Jan=202026=2011:27:03=20+0800=0ASubject:=20[PATCH=20=
v3=201/2]=20tablecmds:=20reject=20CLUSTER=20ON=20for=20partitioned=20=
tables=0A=20earlier=0A=0AALTER=20TABLE=20...=20CLUSTER=20ON=20and=20SET=20=
WITHOUT=20CLUSTER=20are=20not=20supported=20for=0Apartitioned=20tables=20=
and=20already=20fail=20today,=20but=20only=20at=20exec=20time.=0A=0A=
Reject=20these=20commands=20earlier=20via=20ATSimplePermissions(),=20=
matching=20the=0Ahandling=20of=20other=20unsupported=20ALTER=20TABLE=20=
actions=20on=20partitioned=20tables=0A(such=20as=20SET=20LOGGED=20/=20=
SET=20UNLOGGED).=20This=20centralizes=20the=20relation-kind=0Acheck=20in=20=
the=20ALTER=20TABLE=20preparation=20phase,=20improving=20consistency=20=
and=0Amaintainability.=0A=0AAs=20a=20result,=20partitioned=20tables=20=
now=20report=20the=20standard=20ATSimplePermissions()=0Aerror=20for=20=
unsupported=20ALTER=20TABLE=20actions.=0A=0AAuthor:=20Chao=20Li=20=
<lic@highgo.com>=0A---=0A=20src/backend/commands/tablecmds.c=20=20=20=20=20=
=20|=202=20+-=0A=20src/test/regress/expected/cluster.out=20|=206=20=
++++--=0A=202=20files=20changed,=205=20insertions(+),=203=20deletions(-)=0A=
=0Adiff=20--git=20a/src/backend/commands/tablecmds.c=20=
b/src/backend/commands/tablecmds.c=0Aindex=20f976c0e5c7e..3e9f62e9d37=20=
100644=0A---=20a/src/backend/commands/tablecmds.c=0A+++=20=
b/src/backend/commands/tablecmds.c=0A@@=20-5142,7=20+5142,7=20@@=20=
ATPrepCmd(List=20**wqueue,=20Relation=20rel,=20AlterTableCmd=20*cmd,=0A=20=
=09=09case=20AT_ClusterOn:=09=09/*=20CLUSTER=20ON=20*/=0A=20=09=09case=20=
AT_DropCluster:=09/*=20SET=20WITHOUT=20CLUSTER=20*/=0A=20=09=09=09=
ATSimplePermissions(cmd->subtype,=20rel,=0A-=09=09=09=09=09=09=09=09=
ATT_TABLE=20|=20ATT_PARTITIONED_TABLE=20|=20ATT_MATVIEW);=0A+=09=09=09=09=
=09=09=09=09ATT_TABLE=20|=20ATT_MATVIEW);=0A=20=09=09=09/*=20These=20=
commands=20never=20recurse=20*/=0A=20=09=09=09/*=20No=20command-specific=20=
prep=20needed=20*/=0A=20=09=09=09pass=20=3D=20AT_PASS_MISC;=0Adiff=20=
--git=20a/src/test/regress/expected/cluster.out=20=
b/src/test/regress/expected/cluster.out=0Aindex=20=
4d40a6809ab..07c52e647f7=20100644=0A---=20=
a/src/test/regress/expected/cluster.out=0A+++=20=
b/src/test/regress/expected/cluster.out=0A@@=20-492,9=20+492,11=20@@=20=
Number=20of=20partitions:=203=20(Use=20\d+=20to=20list=20them.)=0A=20=
CLUSTER=20clstrpart;=0A=20ERROR:=20=20there=20is=20no=20previously=20=
clustered=20index=20for=20table=20"clstrpart"=0A=20ALTER=20TABLE=20=
clstrpart=20SET=20WITHOUT=20CLUSTER;=0A-ERROR:=20=20cannot=20mark=20=
index=20clustered=20in=20partitioned=20table=0A+ERROR:=20=20ALTER=20=
action=20SET=20WITHOUT=20CLUSTER=20cannot=20be=20performed=20on=20=
relation=20"clstrpart"=0A+DETAIL:=20=20This=20operation=20is=20not=20=
supported=20for=20partitioned=20tables.=0A=20ALTER=20TABLE=20clstrpart=20=
CLUSTER=20ON=20clstrpart_idx;=0A-ERROR:=20=20cannot=20mark=20index=20=
clustered=20in=20partitioned=20table=0A+ERROR:=20=20ALTER=20action=20=
CLUSTER=20ON=20cannot=20be=20performed=20on=20relation=20"clstrpart"=0A=
+DETAIL:=20=20This=20operation=20is=20not=20supported=20for=20=
partitioned=20tables.=0A=20DROP=20TABLE=20clstrpart;=0A=20--=20Ownership=20=
of=20partitions=20is=20checked=0A=20CREATE=20TABLE=20ptnowner(i=20int=20=
unique)=20PARTITION=20BY=20LIST=20(i);=0A--=20=0A2.50.1=20(Apple=20=
Git-155)=0A=0A=

--Apple-Mail=_0C148671-6739-4850-B3C9-3A08AEBF678A
Content-Disposition: attachment;
	filename=v3-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v3-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=201c690132583361730b4ebd9cffb63cb53ee643c4=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20"Chao=20Li=20(Evan)"=20<lic@highgo.com>=0A=
Date:=20Thu,=2022=20Jan=202026=2016:47:58=20+0800=0ASubject:=20[PATCH=20=
v3=202/2]=20tablecmds:=20reject=20INHERIT=20/=20NO=20INHERIT=20for=20=
partitioned=0A=20tables=20earlier=0A=0AALTER=20TABLE=20...=20INHERIT=20=
and=20NO=20INHERIT=20are=20not=20supported=20for=20partitioned=0Atables=20=
and=20already=20fail=20today,=20but=20only=20via=20ad-hoc=20checks=20in=20=
command-=0Aspecific=20preparation=20code=20or=20later=20at=20execution=20=
time.=0A=0AReject=20these=20commands=20earlier=20via=20=
ATSimplePermissions(),=20matching=20the=0Ahandling=20of=20other=20=
unsupported=20ALTER=20TABLE=20actions=20on=20partitioned=20tables.=0A=
This=20centralizes=20relation-kind=20checks=20in=20the=20common=20=
permission=20framework=0Aand=20produces=20the=20standard,=20consistent=20=
error=20message.=0A=0AWhile=20doing=20this,=20fix=20two=20related=20=
issues=20in=20the=20preparation=20logic:=0A-=20The=20header=20comment=20=
of=20ATPrepAddInherit=20was=20a=20stale=20copy-paste=20that=0A=20=20=
described=20behavior=20no=20longer=20implemented=20there.=0A-=20NO=20=
INHERIT=20previously=20didn't=20use=20ATPrepAddInherit,=20causing=20it=20=
to=20reach=0A=20=20execution-time=20checks=20unnecessarily.=20=
Consolidate=20preparation=20into=20a=0A=20=20shared=20helper=20used=20by=20=
both=20INHERIT=20and=20NO=20INHERIT,=20ensuring=20both=20are=0A=20=20=
rejected=20uniformly=20and=20as=20early=20as=20possible.=0A=0AAuthor:=20=
Chao=20Li=20<lic@highgo.com>=0A---=0A=20src/backend/commands/tablecmds.c=20=
=20=20=20=20=20=20=20=20=20|=2022=20++++++++--------------=0A=20=
src/test/regress/expected/alter_table.out=20|=20=203=20++-=0A=202=20=
files=20changed,=2010=20insertions(+),=2015=20deletions(-)=0A=0Adiff=20=
--git=20a/src/backend/commands/tablecmds.c=20=
b/src/backend/commands/tablecmds.c=0Aindex=203e9f62e9d37..a255edd9531=20=
100644=0A---=20a/src/backend/commands/tablecmds.c=0A+++=20=
b/src/backend/commands/tablecmds.c=0A@@=20-686,7=20+686,7=20@@=20static=20=
void=20ATExecEnableDisableTrigger(Relation=20rel,=20const=20char=20=
*trigname,=0A=20=09=09=09=09=09=09=09=09=09=20=20=20LOCKMODE=20=
lockmode);=0A=20static=20void=20ATExecEnableDisableRule(Relation=20rel,=20=
const=20char=20*rulename,=0A=20=09=09=09=09=09=09=09=09=09char=20=
fires_when,=20LOCKMODE=20lockmode);=0A-static=20void=20=
ATPrepAddInherit(Relation=20child_rel);=0A+static=20void=20=
ATPrepAddDropInherit(Relation=20child_rel);=0A=20static=20ObjectAddress=20=
ATExecAddInherit(Relation=20child_rel,=20RangeVar=20*parent,=20LOCKMODE=20=
lockmode);=0A=20static=20ObjectAddress=20ATExecDropInherit(Relation=20=
rel,=20RangeVar=20*parent,=20LOCKMODE=20lockmode);=0A=20static=20void=20=
drop_parent_dependency(Oid=20relid,=20Oid=20refclassid,=20Oid=20=
refobjid,=0A@@=20-5194,15=20+5194,16=20@@=20ATPrepCmd(List=20**wqueue,=20=
Relation=20rel,=20AlterTableCmd=20*cmd,=0A=20=09=09=09break;=0A=20=09=09=
case=20AT_AddInherit:=09=09/*=20INHERIT=20*/=0A=20=09=09=09=
ATSimplePermissions(cmd->subtype,=20rel,=0A-=09=09=09=09=09=09=09=09=
ATT_TABLE=20|=20ATT_PARTITIONED_TABLE=20|=20ATT_FOREIGN_TABLE);=0A+=09=09=
=09=09=09=09=09=09ATT_TABLE=20|=20ATT_FOREIGN_TABLE);=0A=20=09=09=09/*=20=
This=20command=20never=20recurses=20*/=0A-=09=09=09=
ATPrepAddInherit(rel);=0A+=09=09=09ATPrepAddDropInherit(rel);=0A=20=09=09=
=09pass=20=3D=20AT_PASS_MISC;=0A=20=09=09=09break;=0A=20=09=09case=20=
AT_DropInherit:=09/*=20NO=20INHERIT=20*/=0A=20=09=09=09=
ATSimplePermissions(cmd->subtype,=20rel,=0A-=09=09=09=09=09=09=09=09=
ATT_TABLE=20|=20ATT_PARTITIONED_TABLE=20|=20ATT_FOREIGN_TABLE);=0A+=09=09=
=09=09=09=09=09=09ATT_TABLE=20|=20ATT_FOREIGN_TABLE);=0A=20=09=09=09/*=20=
This=20command=20never=20recurses=20*/=0A+=09=09=09=
ATPrepAddDropInherit(rel);=0A=20=09=09=09/*=20No=20command-specific=20=
prep=20needed=20*/=0A=20=09=09=09pass=20=3D=20AT_PASS_MISC;=0A=20=09=09=09=
break;=0A@@=20-17259,14=20+17260,12=20@@=20=
ATExecEnableDisableRule(Relation=20rel,=20const=20char=20*rulename,=0A=20=
}=0A=20=0A=20/*=0A-=20*=20ALTER=20TABLE=20INHERIT=0A+=20*=20ALTER=20=
TABLE=20INHERIT=20/=20NO=20INHERIT=0A=20=20*=0A-=20*=20Add=20a=20parent=20=
to=20the=20child's=20parents.=20This=20verifies=20that=20all=20the=20=
columns=20and=0A-=20*=20check=20constraints=20of=20the=20parent=20appear=20=
in=20the=20child=20and=20that=20they=20have=20the=0A-=20*=20same=20data=20=
types=20and=20expressions.=0A+=20*=20A=20child=20table=20cannot=20be=20a=20=
typed=20table=20or=20a=20partition.=0A=20=20*/=0A=20static=20void=0A=
-ATPrepAddInherit(Relation=20child_rel)=0A+ATPrepAddDropInherit(Relation=20=
child_rel)=0A=20{=0A=20=09if=20(child_rel->rd_rel->reloftype)=0A=20=09=09=
ereport(ERROR,=0A@@=20-17277,11=20+17276,6=20@@=20=
ATPrepAddInherit(Relation=20child_rel)=0A=20=09=09ereport(ERROR,=0A=20=09=
=09=09=09(errcode(ERRCODE_WRONG_OBJECT_TYPE),=0A=20=09=09=09=09=20=
errmsg("cannot=20change=20inheritance=20of=20a=20partition")));=0A-=0A-=09=
if=20(child_rel->rd_rel->relkind=20=3D=3D=20RELKIND_PARTITIONED_TABLE)=0A=
-=09=09ereport(ERROR,=0A-=09=09=09=09=
(errcode(ERRCODE_WRONG_OBJECT_TYPE),=0A-=09=09=09=09=20errmsg("cannot=20=
change=20inheritance=20of=20partitioned=20table")));=0A=20}=0A=20=0A=20=
/*=0Adiff=20--git=20a/src/test/regress/expected/alter_table.out=20=
b/src/test/regress/expected/alter_table.out=0Aindex=20=
ac1a7345d0f..b8150aab08b=20100644=0A---=20=
a/src/test/regress/expected/alter_table.out=0A+++=20=
b/src/test/regress/expected/alter_table.out=0A@@=20-3998,7=20+3998,8=20=
@@=20CREATE=20TABLE=20nonpartitioned=20(=0A=20=09b=20int=0A=20);=0A=20=
ALTER=20TABLE=20partitioned=20INHERIT=20nonpartitioned;=0A-ERROR:=20=20=
cannot=20change=20inheritance=20of=20partitioned=20table=0A+ERROR:=20=20=
ALTER=20action=20INHERIT=20cannot=20be=20performed=20on=20relation=20=
"partitioned"=0A+DETAIL:=20=20This=20operation=20is=20not=20supported=20=
for=20partitioned=20tables.=0A=20ALTER=20TABLE=20nonpartitioned=20=
INHERIT=20partitioned;=0A=20ERROR:=20=20cannot=20inherit=20from=20=
partitioned=20table=20"partitioned"=0A=20--=20cannot=20add=20NO=20=
INHERIT=20constraint=20to=20partitioned=20tables=0A--=20=0A2.50.1=20=
(Apple=20Git-155)=0A=0A=

--Apple-Mail=_0C148671-6739-4850-B3C9-3A08AEBF678A--