public inbox for [email protected]
help / color / mirror / Atom feedRe: tablecmds: reject CLUSTER ON for partitioned tables earlier
11+ messages / 3 participants
[nested] [flat]
* Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
@ 2026-01-22 09:01 Chao Li <[email protected]>
2026-01-22 23:30 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Chao Li @ 2026-01-22 09:01 UTC (permalink / raw)
To: Postgres hackers <[email protected]>
On Jan 21, 2026, at 11:55, Chao Li <[email protected]> wrote:
Hi Hacker,
I noticed this while working other patches related to “ALTER TABLE”.
“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.
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.
As a result, the error reported for partitioned tables changes:
Before the patch:
```
evantest=# ALTER TABLE p_test CLUSTER ON idx_p_test_id;
ERROR: cannot mark index clustered in partitioned table
```
With the patch:
```
evantest=# 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.
```
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
<v1-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch>
Applying the same change to INHERIT/NO INHeRIT in v2-0002. Other than that,
fixing 2 more things for INHERIT/NO INHERIT:
* The header comment of ATPrepAddInherit() was a copy-paste mistake, it
described something totally unrelated.
* NO INHERIT didn’t call ATPrepAddInherit() to check early, so it had to go
deeper and run unnecessary checks.
Basically, 0001 and 0002 do the same thing on two sub-commands. If
accepted, they can be squashed.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v2-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch (2.6K, 3-v2-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch)
download | inline diff:
From 75a3d94296e83ec5cc82b98f606227503f2c575e Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Wed, 21 Jan 2026 11:27:03 +0800
Subject: [PATCH v2 1/2] tablecmds: reject CLUSTER ON for partitioned tables
earlier
ALTER TABLE ... CLUSTER ON and SET WITHOUT CLUSTER are not supported for
partitioned tables and already fail today, but only at exec time.
Reject these commands earlier via ATSimplePermissions(), matching the
handling of other unsupported ALTER TABLE actions on partitioned tables
(such as SET LOGGED / SET UNLOGGED). This centralizes the relation-kind
check in the ALTER TABLE preparation phase, improving consistency and
maintainability.
As a result, partitioned tables now report the standard ATSimplePermissions()
error for unsupported ALTER TABLE actions.
Author: Chao Li <[email protected]>
---
src/backend/commands/tablecmds.c | 2 +-
src/test/regress/expected/cluster.out | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f976c0e5c7e..3e9f62e9d37 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5142,7 +5142,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_ClusterOn: /* CLUSTER ON */
case AT_DropCluster: /* SET WITHOUT CLUSTER */
ATSimplePermissions(cmd->subtype, rel,
- ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_MATVIEW);
+ ATT_TABLE | ATT_MATVIEW);
/* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index 4d40a6809ab..07c52e647f7 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -492,9 +492,11 @@ Number of partitions: 3 (Use \d+ to list them.)
CLUSTER clstrpart;
ERROR: there is no previously clustered index for table "clstrpart"
ALTER TABLE clstrpart SET WITHOUT CLUSTER;
-ERROR: cannot mark index clustered in partitioned table
+ERROR: ALTER action SET WITHOUT CLUSTER cannot be performed on relation "clstrpart"
+DETAIL: This operation is not supported for partitioned tables.
ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
-ERROR: cannot mark index clustered in partitioned table
+ERROR: ALTER action CLUSTER ON cannot be performed on relation "clstrpart"
+DETAIL: This operation is not supported for partitioned tables.
DROP TABLE clstrpart;
-- Ownership of partitions is checked
CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i);
--
2.50.1 (Apple Git-155)
[application/octet-stream] v2-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch (4.5K, 4-v2-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch)
download | inline diff:
From 1c690132583361730b4ebd9cffb63cb53ee643c4 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Thu, 22 Jan 2026 16:47:58 +0800
Subject: [PATCH v2 2/2] tablecmds: reject INHERIT / NO INHERIT for partitioned
tables earlier
ALTER TABLE ... INHERIT and NO INHERIT are not supported for partitioned
tables and already fail today, but only via ad-hoc checks in command-
specific preparation code or later at execution time.
Reject these commands earlier via ATSimplePermissions(), matching the
handling of other unsupported ALTER TABLE actions on partitioned tables.
This centralizes relation-kind checks in the common permission framework
and produces the standard, consistent error message.
While doing this, fix two related issues in the preparation logic:
- The header comment of ATPrepAddInherit was a stale copy-paste that
described behavior no longer implemented there.
- NO INHERIT previously didn't use ATPrepAddInherit, causing it to reach
execution-time checks unnecessarily. Consolidate preparation into a
shared helper used by both INHERIT and NO INHERIT, ensuring both are
rejected uniformly and as early as possible.
Author: Chao Li <[email protected]>
---
src/backend/commands/tablecmds.c | 22 ++++++++--------------
src/test/regress/expected/alter_table.out | 3 ++-
2 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3e9f62e9d37..a255edd9531 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -686,7 +686,7 @@ static void ATExecEnableDisableTrigger(Relation rel, const char *trigname,
LOCKMODE lockmode);
static void ATExecEnableDisableRule(Relation rel, const char *rulename,
char fires_when, LOCKMODE lockmode);
-static void ATPrepAddInherit(Relation child_rel);
+static void ATPrepAddDropInherit(Relation child_rel);
static ObjectAddress ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode);
static ObjectAddress ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode);
static void drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid,
@@ -5194,15 +5194,16 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_AddInherit: /* INHERIT */
ATSimplePermissions(cmd->subtype, rel,
- ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_FOREIGN_TABLE);
+ ATT_TABLE | ATT_FOREIGN_TABLE);
/* This command never recurses */
- ATPrepAddInherit(rel);
+ ATPrepAddDropInherit(rel);
pass = AT_PASS_MISC;
break;
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 */
+ ATPrepAddDropInherit(rel);
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
@@ -17259,14 +17260,12 @@ ATExecEnableDisableRule(Relation rel, const char *rulename,
}
/*
- * ALTER TABLE INHERIT
+ * ALTER TABLE INHERIT / NO 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.
+ * A child table cannot be a typed table or a partition.
*/
static void
-ATPrepAddInherit(Relation child_rel)
+ATPrepAddDropInherit(Relation child_rel)
{
if (child_rel->rd_rel->reloftype)
ereport(ERROR,
@@ -17277,11 +17276,6 @@ ATPrepAddInherit(Relation child_rel)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot change inheritance of a partition")));
-
- if (child_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot change inheritance of partitioned table")));
}
/*
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index ac1a7345d0f..b8150aab08b 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3998,7 +3998,8 @@ CREATE TABLE nonpartitioned (
b int
);
ALTER TABLE partitioned INHERIT nonpartitioned;
-ERROR: cannot change inheritance of partitioned table
+ERROR: ALTER action INHERIT cannot be performed on relation "partitioned"
+DETAIL: This operation is not supported for partitioned tables.
ALTER TABLE nonpartitioned INHERIT partitioned;
ERROR: cannot inherit from partitioned table "partitioned"
-- cannot add NO INHERIT constraint to partitioned tables
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
2026-01-22 09:01 Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
@ 2026-01-22 23:30 ` Chao Li <[email protected]>
2026-01-23 00:54 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Chao Li @ 2026-01-22 23:30 UTC (permalink / raw)
To: Postgres hackers <[email protected]>
> On Jan 22, 2026, at 17:01, Chao Li <[email protected]> wrote:
>
>
>
>> On Jan 21, 2026, at 11:55, Chao Li <[email protected]> wrote:
>>
>> Hi Hacker,
>>
>> I noticed this while working other patches related to “ALTER TABLE”.
>>
>> “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.
>>
>> 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.
>>
>> As a result, the error reported for partitioned tables changes:
>>
>> Before the patch:
>> ```
>> evantest=# ALTER TABLE p_test CLUSTER ON idx_p_test_id;
>> ERROR: cannot mark index clustered in partitioned table
>> ```
>>
>> With the patch:
>> ```
>> evantest=# 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.
>> ```
>>
>> Best regards,
>> --
>> Chao Li (Evan)
>> HighGo Software Co., Ltd.
>> https://www.highgo.com/
>>
>>
>>
>>
>> <v1-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch>
>
>
>
> Applying the same change to INHERIT/NO INHeRIT in v2-0002. Other than that, fixing 2 more things for INHERIT/NO INHERIT:
>
> * The header comment of ATPrepAddInherit() was a copy-paste mistake, it described something totally unrelated.
> * NO INHERIT didn’t call ATPrepAddInherit() to check early, so it had to go deeper and run unnecessary checks.
>
> Basically, 0001 and 0002 do the same thing on two sub-commands. If accepted, they can be squashed.
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
> <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/
Attachments:
[application/octet-stream] v3-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch (2.6K, 2-v3-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch)
download | inline diff:
From 75a3d94296e83ec5cc82b98f606227503f2c575e Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Wed, 21 Jan 2026 11:27:03 +0800
Subject: [PATCH v3 1/2] tablecmds: reject CLUSTER ON for partitioned tables
earlier
ALTER TABLE ... CLUSTER ON and SET WITHOUT CLUSTER are not supported for
partitioned tables and already fail today, but only at exec time.
Reject these commands earlier via ATSimplePermissions(), matching the
handling of other unsupported ALTER TABLE actions on partitioned tables
(such as SET LOGGED / SET UNLOGGED). This centralizes the relation-kind
check in the ALTER TABLE preparation phase, improving consistency and
maintainability.
As a result, partitioned tables now report the standard ATSimplePermissions()
error for unsupported ALTER TABLE actions.
Author: Chao Li <[email protected]>
---
src/backend/commands/tablecmds.c | 2 +-
src/test/regress/expected/cluster.out | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f976c0e5c7e..3e9f62e9d37 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5142,7 +5142,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_ClusterOn: /* CLUSTER ON */
case AT_DropCluster: /* SET WITHOUT CLUSTER */
ATSimplePermissions(cmd->subtype, rel,
- ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_MATVIEW);
+ ATT_TABLE | ATT_MATVIEW);
/* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index 4d40a6809ab..07c52e647f7 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -492,9 +492,11 @@ Number of partitions: 3 (Use \d+ to list them.)
CLUSTER clstrpart;
ERROR: there is no previously clustered index for table "clstrpart"
ALTER TABLE clstrpart SET WITHOUT CLUSTER;
-ERROR: cannot mark index clustered in partitioned table
+ERROR: ALTER action SET WITHOUT CLUSTER cannot be performed on relation "clstrpart"
+DETAIL: This operation is not supported for partitioned tables.
ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
-ERROR: cannot mark index clustered in partitioned table
+ERROR: ALTER action CLUSTER ON cannot be performed on relation "clstrpart"
+DETAIL: This operation is not supported for partitioned tables.
DROP TABLE clstrpart;
-- Ownership of partitions is checked
CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i);
--
2.50.1 (Apple Git-155)
[application/octet-stream] v3-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch (4.5K, 3-v3-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch)
download | inline diff:
From 1c690132583361730b4ebd9cffb63cb53ee643c4 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Thu, 22 Jan 2026 16:47:58 +0800
Subject: [PATCH v3 2/2] tablecmds: reject INHERIT / NO INHERIT for partitioned
tables earlier
ALTER TABLE ... INHERIT and NO INHERIT are not supported for partitioned
tables and already fail today, but only via ad-hoc checks in command-
specific preparation code or later at execution time.
Reject these commands earlier via ATSimplePermissions(), matching the
handling of other unsupported ALTER TABLE actions on partitioned tables.
This centralizes relation-kind checks in the common permission framework
and produces the standard, consistent error message.
While doing this, fix two related issues in the preparation logic:
- The header comment of ATPrepAddInherit was a stale copy-paste that
described behavior no longer implemented there.
- NO INHERIT previously didn't use ATPrepAddInherit, causing it to reach
execution-time checks unnecessarily. Consolidate preparation into a
shared helper used by both INHERIT and NO INHERIT, ensuring both are
rejected uniformly and as early as possible.
Author: Chao Li <[email protected]>
---
src/backend/commands/tablecmds.c | 22 ++++++++--------------
src/test/regress/expected/alter_table.out | 3 ++-
2 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3e9f62e9d37..a255edd9531 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -686,7 +686,7 @@ static void ATExecEnableDisableTrigger(Relation rel, const char *trigname,
LOCKMODE lockmode);
static void ATExecEnableDisableRule(Relation rel, const char *rulename,
char fires_when, LOCKMODE lockmode);
-static void ATPrepAddInherit(Relation child_rel);
+static void ATPrepAddDropInherit(Relation child_rel);
static ObjectAddress ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode);
static ObjectAddress ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode);
static void drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid,
@@ -5194,15 +5194,16 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_AddInherit: /* INHERIT */
ATSimplePermissions(cmd->subtype, rel,
- ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_FOREIGN_TABLE);
+ ATT_TABLE | ATT_FOREIGN_TABLE);
/* This command never recurses */
- ATPrepAddInherit(rel);
+ ATPrepAddDropInherit(rel);
pass = AT_PASS_MISC;
break;
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 */
+ ATPrepAddDropInherit(rel);
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
@@ -17259,14 +17260,12 @@ ATExecEnableDisableRule(Relation rel, const char *rulename,
}
/*
- * ALTER TABLE INHERIT
+ * ALTER TABLE INHERIT / NO 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.
+ * A child table cannot be a typed table or a partition.
*/
static void
-ATPrepAddInherit(Relation child_rel)
+ATPrepAddDropInherit(Relation child_rel)
{
if (child_rel->rd_rel->reloftype)
ereport(ERROR,
@@ -17277,11 +17276,6 @@ ATPrepAddInherit(Relation child_rel)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot change inheritance of a partition")));
-
- if (child_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot change inheritance of partitioned table")));
}
/*
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index ac1a7345d0f..b8150aab08b 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3998,7 +3998,8 @@ CREATE TABLE nonpartitioned (
b int
);
ALTER TABLE partitioned INHERIT nonpartitioned;
-ERROR: cannot change inheritance of partitioned table
+ERROR: ALTER action INHERIT cannot be performed on relation "partitioned"
+DETAIL: This operation is not supported for partitioned tables.
ALTER TABLE nonpartitioned INHERIT partitioned;
ERROR: cannot inherit from partitioned table "partitioned"
-- cannot add NO INHERIT constraint to partitioned tables
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
2026-01-22 09:01 Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-22 23:30 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
@ 2026-01-23 00:54 ` Chao Li <[email protected]>
2026-01-23 07:43 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Chao Li @ 2026-01-23 00:54 UTC (permalink / raw)
To: Postgres hackers <[email protected]>
> On Jan 23, 2026, at 07:30, Chao Li <[email protected]> wrote:
>
>
>
>> On Jan 22, 2026, at 17:01, Chao Li <[email protected]> wrote:
>>
>>
>>
>>> On Jan 21, 2026, at 11:55, Chao Li <[email protected]> wrote:
>>>
>>> Hi Hacker,
>>>
>>> I noticed this while working other patches related to “ALTER TABLE”.
>>>
>>> “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.
>>>
>>> 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.
>>>
>>> As a result, the error reported for partitioned tables changes:
>>>
>>> Before the patch:
>>> ```
>>> evantest=# ALTER TABLE p_test CLUSTER ON idx_p_test_id;
>>> ERROR: cannot mark index clustered in partitioned table
>>> ```
>>>
>>> With the patch:
>>> ```
>>> evantest=# 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.
>>> ```
>>>
>>> Best regards,
>>> --
>>> Chao Li (Evan)
>>> HighGo Software Co., Ltd.
>>> https://www.highgo.com/
>>>
>>>
>>>
>>>
>>> <v1-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch>
>>
>>
>>
>> Applying the same change to INHERIT/NO INHeRIT in v2-0002. Other than that, fixing 2 more things for INHERIT/NO INHERIT:
>>
>> * The header comment of ATPrepAddInherit() was a copy-paste mistake, it described something totally unrelated.
>> * NO INHERIT didn’t call ATPrepAddInherit() to check early, so it had to go deeper and run unnecessary checks.
>>
>> Basically, 0001 and 0002 do the same thing on two sub-commands. If accepted, they can be squashed.
>>
>> Best regards,
>> --
>> Chao Li (Evan)
>> HighGo Software Co., Ltd.
>> https://www.highgo.com/
>>
>> <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/
>
>
>
>
> <v3-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch><v3-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch>
Please ignore v3 that was badly generated.
PFA v4:
> 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/
Attachments:
[application/octet-stream] v4-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch (2.7K, 2-v4-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch)
download | inline diff:
From 7426bb3c6a8ceaed329dc34e5ef81da071dcc25f Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Wed, 21 Jan 2026 11:27:03 +0800
Subject: [PATCH v4 1/2] tablecmds: reject CLUSTER ON for partitioned tables
earlier
ALTER TABLE ... CLUSTER ON and SET WITHOUT CLUSTER are not supported for
partitioned tables and already fail today, but only at exec time.
Reject these commands earlier via ATSimplePermissions(), matching the
handling of other unsupported ALTER TABLE actions on partitioned tables
(such as SET LOGGED / SET UNLOGGED). This centralizes the relation-kind
check in the ALTER TABLE preparation phase, improving consistency and
maintainability.
As a result, partitioned tables now report the standard ATSimplePermissions()
error for unsupported ALTER TABLE actions.
Author: Chao Li <[email protected]>
Discussion: https://postgr.es/m/CAEoWx2kggo1N2kDH6OSfXHL_5gKg3DqQ0PdNuL4LH4XSTKJ3-g@mail.gmail.com
---
src/backend/commands/tablecmds.c | 2 +-
src/test/regress/expected/cluster.out | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f976c0e5c7e..3e9f62e9d37 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5142,7 +5142,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_ClusterOn: /* CLUSTER ON */
case AT_DropCluster: /* SET WITHOUT CLUSTER */
ATSimplePermissions(cmd->subtype, rel,
- ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_MATVIEW);
+ ATT_TABLE | ATT_MATVIEW);
/* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index 4d40a6809ab..07c52e647f7 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -492,9 +492,11 @@ Number of partitions: 3 (Use \d+ to list them.)
CLUSTER clstrpart;
ERROR: there is no previously clustered index for table "clstrpart"
ALTER TABLE clstrpart SET WITHOUT CLUSTER;
-ERROR: cannot mark index clustered in partitioned table
+ERROR: ALTER action SET WITHOUT CLUSTER cannot be performed on relation "clstrpart"
+DETAIL: This operation is not supported for partitioned tables.
ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
-ERROR: cannot mark index clustered in partitioned table
+ERROR: ALTER action CLUSTER ON cannot be performed on relation "clstrpart"
+DETAIL: This operation is not supported for partitioned tables.
DROP TABLE clstrpart;
-- Ownership of partitions is checked
CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i);
--
2.50.1 (Apple Git-155)
[application/octet-stream] v4-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch (5.0K, 3-v4-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch)
download | inline diff:
From 617d6f847dcd5277341013cf7c966eac5cc98bc5 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Thu, 22 Jan 2026 16:47:58 +0800
Subject: [PATCH v4 2/2] tablecmds: reject INHERIT / NO INHERIT for partitioned
tables earlier
ALTER TABLE ... INHERIT and NO INHERIT are not supported for partitioned
tables and already fail today, but only via ad-hoc checks in command-
specific preparation code or later at execution time.
Reject these commands earlier via ATSimplePermissions(), matching the
handling of other unsupported ALTER TABLE actions on partitioned tables.
This centralizes relation-kind checks in the common permission framework
and produces the standard, consistent error message.
While doing this, fix two related issues in the preparation logic:
- The header comment of ATPrepAddInherit was a stale copy-paste that
described behavior no longer implemented there.
- NO INHERIT previously didn't use ATPrepAddInherit, causing it to reach
execution-time checks unnecessarily. Consolidate preparation into a
shared helper used by both INHERIT and NO INHERIT, ensuring both are
rejected uniformly and as early as possible.
Author: Chao Li <[email protected]>
Discussion: https://postgr.es/m/CAEoWx2kggo1N2kDH6OSfXHL_5gKg3DqQ0PdNuL4LH4XSTKJ3-g@mail.gmail.com
---
src/backend/commands/tablecmds.c | 30 ++++++++++++-----------
src/test/regress/expected/alter_table.out | 3 ++-
2 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3e9f62e9d37..2cf5269e73d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -686,7 +686,7 @@ static void ATExecEnableDisableTrigger(Relation rel, const char *trigname,
LOCKMODE lockmode);
static void ATExecEnableDisableRule(Relation rel, const char *rulename,
char fires_when, LOCKMODE lockmode);
-static void ATPrepAddInherit(Relation child_rel);
+static void ATPrepChangeInherit(Relation child_rel);
static ObjectAddress ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode);
static ObjectAddress ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode);
static void drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid,
@@ -5194,15 +5194,16 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_AddInherit: /* INHERIT */
ATSimplePermissions(cmd->subtype, rel,
- ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_FOREIGN_TABLE);
+ ATT_TABLE | ATT_FOREIGN_TABLE);
/* This command never recurses */
- ATPrepAddInherit(rel);
+ ATPrepChangeInherit(rel);
pass = AT_PASS_MISC;
break;
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 */
pass = AT_PASS_MISC;
break;
@@ -17259,14 +17260,14 @@ ATExecEnableDisableRule(Relation rel, const char *rulename,
}
/*
- * ALTER TABLE INHERIT
+ * Prepare to change inheritance.
*
- * 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.
+ * A child table cannot be a typed table or a partition. We don't need to check
+ * partitioned table here, because it has been blocked by ATSimplePermissions in
+ * ATPrepCmd.
*/
static void
-ATPrepAddInherit(Relation child_rel)
+ATPrepChangeInherit(Relation child_rel)
{
if (child_rel->rd_rel->reloftype)
ereport(ERROR,
@@ -17277,14 +17278,15 @@ ATPrepAddInherit(Relation child_rel)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot change inheritance of a partition")));
-
- if (child_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot change inheritance of partitioned table")));
}
/*
+ * 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.
*/
static ObjectAddress
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index ac1a7345d0f..b8150aab08b 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3998,7 +3998,8 @@ CREATE TABLE nonpartitioned (
b int
);
ALTER TABLE partitioned INHERIT nonpartitioned;
-ERROR: cannot change inheritance of partitioned table
+ERROR: ALTER action INHERIT cannot be performed on relation "partitioned"
+DETAIL: This operation is not supported for partitioned tables.
ALTER TABLE nonpartitioned INHERIT partitioned;
ERROR: cannot inherit from partitioned table "partitioned"
-- cannot add NO INHERIT constraint to partitioned tables
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
2026-01-22 09:01 Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-22 23:30 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 00:54 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
@ 2026-01-23 07:43 ` Zsolt Parragi <[email protected]>
2026-01-23 07:53 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-26 07:10 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
0 siblings, 2 replies; 11+ messages in thread
From: Zsolt Parragi @ 2026-01-23 07:43 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: Postgres hackers <[email protected]>
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.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
2026-01-22 09:01 Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-22 23:30 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 00:54 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 07:43 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
@ 2026-01-23 07:53 ` Chao Li <[email protected]>
1 sibling, 0 replies; 11+ messages in thread
From: Chao Li @ 2026-01-23 07:53 UTC (permalink / raw)
To: Zsolt Parragi <[email protected]>; +Cc: Postgres hackers <[email protected]>
> On Jan 23, 2026, at 15:43, Zsolt Parragi <[email protected]> wrote:
>
> 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?
>
Hi Zsolt,
Thank you very much for your review.
I have the other patch for the documentation update, see [1], that is an overall clarification for alter table behaviors against partition tables. Actually, I just found this issue while working on that patch.
I will handle rest of your comments soon.
[1] https://www.postgresql.org/message-id/CAEoWx2%3DmYhCfsnHaN96Qqwq5b0GVS2YgO3zpVqPPRd_iO52wRw%40mail.g...
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
2026-01-22 09:01 Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-22 23:30 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 00:54 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 07:43 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
@ 2026-01-26 07:10 ` Chao Li <[email protected]>
2026-01-26 17:26 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
1 sibling, 1 reply; 11+ messages in thread
From: Chao Li @ 2026-01-26 07:10 UTC (permalink / raw)
To: Zsolt Parragi <[email protected]>; +Cc: Postgres hackers <[email protected]>
> On Jan 23, 2026, at 15:43, Zsolt Parragi <[email protected]> wrote:
>
> 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
Added a test case.
>
> 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.
Deleted the comment.
>
> 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.
> */
>
Deleted the whitespace.
> tablecmds.c:17860 - this check in ATExecDropInherit is now redundant,
> since we already have it in ATPrepChangeInherit
No, the check is not redundant. It checks for child partitions, while ATPrepChangeInherit only blocks partitioned tables.
>
>> 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.
The portioned table check was added to mark_index_clustered with 05fb5d6. In the commit, the test case "ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;” was added as well, so my best guess the check is now no longer needed. I tried to remove the check, and “make check” still passes.
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.
So, I would leave the check there, maybe use a separate discussion for removal of the check.
PFA v5.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v5-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch (2.7K, 2-v5-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch)
download | inline diff:
From 5489e109c0a06cd437d33f72d603e6e01825e794 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Wed, 21 Jan 2026 11:27:03 +0800
Subject: [PATCH v5 1/2] tablecmds: reject CLUSTER ON for partitioned tables
earlier
ALTER TABLE ... CLUSTER ON and SET WITHOUT CLUSTER are not supported for
partitioned tables and already fail today, but only at exec time.
Reject these commands earlier via ATSimplePermissions(), matching the
handling of other unsupported ALTER TABLE actions on partitioned tables
(such as SET LOGGED / SET UNLOGGED). This centralizes the relation-kind
check in the ALTER TABLE preparation phase, improving consistency and
maintainability.
As a result, partitioned tables now report the standard ATSimplePermissions()
error for unsupported ALTER TABLE actions.
Author: Chao Li <[email protected]>
Reviewed-by: Zsolt Parragi <[email protected]>
Discussion: https://postgr.es/m/CAEoWx2kggo1N2kDH6OSfXHL_5gKg3DqQ0PdNuL4LH4XSTKJ3-g@mail.gmail.com
---
src/backend/commands/tablecmds.c | 2 +-
src/test/regress/expected/cluster.out | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f976c0e5c7e..3e9f62e9d37 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5142,7 +5142,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_ClusterOn: /* CLUSTER ON */
case AT_DropCluster: /* SET WITHOUT CLUSTER */
ATSimplePermissions(cmd->subtype, rel,
- ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_MATVIEW);
+ ATT_TABLE | ATT_MATVIEW);
/* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index 4d40a6809ab..07c52e647f7 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -492,9 +492,11 @@ Number of partitions: 3 (Use \d+ to list them.)
CLUSTER clstrpart;
ERROR: there is no previously clustered index for table "clstrpart"
ALTER TABLE clstrpart SET WITHOUT CLUSTER;
-ERROR: cannot mark index clustered in partitioned table
+ERROR: ALTER action SET WITHOUT CLUSTER cannot be performed on relation "clstrpart"
+DETAIL: This operation is not supported for partitioned tables.
ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
-ERROR: cannot mark index clustered in partitioned table
+ERROR: ALTER action CLUSTER ON cannot be performed on relation "clstrpart"
+DETAIL: This operation is not supported for partitioned tables.
DROP TABLE clstrpart;
-- Ownership of partitions is checked
CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i);
--
2.50.1 (Apple Git-155)
[application/octet-stream] v5-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch (6.3K, 3-v5-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch)
download | inline diff:
From 3899168c6e709add69a291aa4f781bf47d86cbb1 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Thu, 22 Jan 2026 16:47:58 +0800
Subject: [PATCH v5 2/2] tablecmds: reject INHERIT / NO INHERIT for partitioned
tables earlier
ALTER TABLE ... INHERIT and NO INHERIT are not supported for partitioned
tables and already fail today, but only via ad-hoc checks in command-
specific preparation code or later at execution time.
Reject these commands earlier via ATSimplePermissions(), matching the
handling of other unsupported ALTER TABLE actions on partitioned tables.
This centralizes relation-kind checks in the common permission framework
and produces the standard, consistent error message.
While doing this, fix two related issues in the preparation logic:
- The header comment of ATPrepAddInherit was a stale copy-paste that
described behavior no longer implemented there.
- NO INHERIT previously didn't use ATPrepAddInherit, causing it to reach
execution-time checks unnecessarily. Consolidate preparation into a
shared helper used by both INHERIT and NO INHERIT, ensuring both are
rejected uniformly and as early as possible.
Author: Chao Li <[email protected]>
Reviewed-by: Zsolt Parragi <[email protected]>
Discussion: https://postgr.es/m/CAEoWx2kggo1N2kDH6OSfXHL_5gKg3DqQ0PdNuL4LH4XSTKJ3-g@mail.gmail.com
---
src/backend/commands/tablecmds.c | 31 ++++++++++++-----------
src/test/regress/expected/alter_table.out | 10 +++++---
src/test/regress/sql/alter_table.sql | 5 ++--
3 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3e9f62e9d37..379f4d4ebaf 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -686,7 +686,7 @@ static void ATExecEnableDisableTrigger(Relation rel, const char *trigname,
LOCKMODE lockmode);
static void ATExecEnableDisableRule(Relation rel, const char *rulename,
char fires_when, LOCKMODE lockmode);
-static void ATPrepAddInherit(Relation child_rel);
+static void ATPrepChangeInherit(Relation child_rel);
static ObjectAddress ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode);
static ObjectAddress ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode);
static void drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid,
@@ -5194,16 +5194,16 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_AddInherit: /* INHERIT */
ATSimplePermissions(cmd->subtype, rel,
- ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_FOREIGN_TABLE);
+ ATT_TABLE | ATT_FOREIGN_TABLE);
/* This command never recurses */
- ATPrepAddInherit(rel);
+ ATPrepChangeInherit(rel);
pass = AT_PASS_MISC;
break;
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 */
- /* No command-specific prep needed */
+ ATPrepChangeInherit(rel);
pass = AT_PASS_MISC;
break;
case AT_AlterConstraint: /* ALTER CONSTRAINT */
@@ -17259,14 +17259,14 @@ ATExecEnableDisableRule(Relation rel, const char *rulename,
}
/*
- * ALTER TABLE INHERIT
+ * Prepare to change inheritance.
*
- * 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.
+ * A child table cannot be a typed table or a partition. We don't need to check
+ * partitioned table here, because it has been blocked by ATSimplePermissions in
+ * ATPrepCmd.
*/
static void
-ATPrepAddInherit(Relation child_rel)
+ATPrepChangeInherit(Relation child_rel)
{
if (child_rel->rd_rel->reloftype)
ereport(ERROR,
@@ -17277,14 +17277,15 @@ ATPrepAddInherit(Relation child_rel)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot change inheritance of a partition")));
-
- if (child_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot change inheritance of partitioned table")));
}
/*
+ * 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.
*/
static ObjectAddress
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index ac1a7345d0f..8e1ee3f1389 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3997,9 +3997,13 @@ CREATE TABLE nonpartitioned (
a int,
b int
);
-ALTER TABLE partitioned INHERIT nonpartitioned;
-ERROR: cannot change inheritance of partitioned table
-ALTER TABLE nonpartitioned INHERIT partitioned;
+ALTER TABLE partitioned INHERIT nonpartitioned; -- fail
+ERROR: ALTER action INHERIT cannot be performed on relation "partitioned"
+DETAIL: This operation is not supported for partitioned tables.
+ALTER TABLE partitioned NO INHERIT nonpartitioned; -- fail
+ERROR: ALTER action NO INHERIT cannot be performed on relation "partitioned"
+DETAIL: This operation is not supported for partitioned tables.
+ALTER TABLE nonpartitioned INHERIT partitioned; -- ok
ERROR: cannot inherit from partitioned table "partitioned"
-- cannot add NO INHERIT constraint to partitioned tables
ALTER TABLE partitioned ADD CONSTRAINT chk_a CHECK (a > 0) NO INHERIT;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 417202430a5..0241e2796c9 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2397,8 +2397,9 @@ CREATE TABLE nonpartitioned (
a int,
b int
);
-ALTER TABLE partitioned INHERIT nonpartitioned;
-ALTER TABLE nonpartitioned INHERIT partitioned;
+ALTER TABLE partitioned INHERIT nonpartitioned; -- fail
+ALTER TABLE partitioned NO INHERIT nonpartitioned; -- fail
+ALTER TABLE nonpartitioned INHERIT partitioned; -- ok
-- cannot add NO INHERIT constraint to partitioned tables
ALTER TABLE partitioned ADD CONSTRAINT chk_a CHECK (a > 0) NO INHERIT;
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
2026-01-22 09:01 Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-22 23:30 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 00:54 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 07:43 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
2026-01-26 07:10 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
@ 2026-01-26 17:26 ` Zsolt Parragi <[email protected]>
2026-01-26 23:13 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Zsolt Parragi @ 2026-01-26 17:26 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: Postgres hackers <[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.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
2026-01-22 09:01 Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-22 23:30 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 00:54 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 07:43 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
2026-01-26 07:10 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-26 17:26 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
@ 2026-01-26 23:13 ` Chao Li <[email protected]>
2026-01-27 07:48 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Michael Paquier <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Chao Li @ 2026-01-26 23:13 UTC (permalink / raw)
To: Zsolt Parragi <[email protected]>; +Cc: Postgres hackers <[email protected]>
> On Jan 27, 2026, at 01:26, Zsolt Parragi <[email protected]> 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/
Attachments:
[application/octet-stream] v6-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch (2.7K, 2-v6-0001-tablecmds-reject-CLUSTER-ON-for-partitioned-table.patch)
download | inline diff:
From 5489e109c0a06cd437d33f72d603e6e01825e794 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Wed, 21 Jan 2026 11:27:03 +0800
Subject: [PATCH v6 1/2] tablecmds: reject CLUSTER ON for partitioned tables
earlier
ALTER TABLE ... CLUSTER ON and SET WITHOUT CLUSTER are not supported for
partitioned tables and already fail today, but only at exec time.
Reject these commands earlier via ATSimplePermissions(), matching the
handling of other unsupported ALTER TABLE actions on partitioned tables
(such as SET LOGGED / SET UNLOGGED). This centralizes the relation-kind
check in the ALTER TABLE preparation phase, improving consistency and
maintainability.
As a result, partitioned tables now report the standard ATSimplePermissions()
error for unsupported ALTER TABLE actions.
Author: Chao Li <[email protected]>
Reviewed-by: Zsolt Parragi <[email protected]>
Discussion: https://postgr.es/m/CAEoWx2kggo1N2kDH6OSfXHL_5gKg3DqQ0PdNuL4LH4XSTKJ3-g@mail.gmail.com
---
src/backend/commands/tablecmds.c | 2 +-
src/test/regress/expected/cluster.out | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f976c0e5c7e..3e9f62e9d37 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5142,7 +5142,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_ClusterOn: /* CLUSTER ON */
case AT_DropCluster: /* SET WITHOUT CLUSTER */
ATSimplePermissions(cmd->subtype, rel,
- ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_MATVIEW);
+ ATT_TABLE | ATT_MATVIEW);
/* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index 4d40a6809ab..07c52e647f7 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -492,9 +492,11 @@ Number of partitions: 3 (Use \d+ to list them.)
CLUSTER clstrpart;
ERROR: there is no previously clustered index for table "clstrpart"
ALTER TABLE clstrpart SET WITHOUT CLUSTER;
-ERROR: cannot mark index clustered in partitioned table
+ERROR: ALTER action SET WITHOUT CLUSTER cannot be performed on relation "clstrpart"
+DETAIL: This operation is not supported for partitioned tables.
ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
-ERROR: cannot mark index clustered in partitioned table
+ERROR: ALTER action CLUSTER ON cannot be performed on relation "clstrpart"
+DETAIL: This operation is not supported for partitioned tables.
DROP TABLE clstrpart;
-- Ownership of partitions is checked
CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i);
--
2.50.1 (Apple Git-155)
[application/octet-stream] v6-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch (6.9K, 3-v6-0002-tablecmds-reject-INHERIT-NO-INHERIT-for-partition.patch)
download | inline diff:
From 9599ea3e3e6ee9b5168c5e373168340a39729205 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Thu, 22 Jan 2026 16:47:58 +0800
Subject: [PATCH v6 2/2] tablecmds: reject INHERIT / NO INHERIT for partitioned
tables earlier
ALTER TABLE ... INHERIT and NO INHERIT are not supported for partitioned
tables and already fail today, but only via ad-hoc checks in command-
specific preparation code or later at execution time.
Reject these commands earlier via ATSimplePermissions(), matching the
handling of other unsupported ALTER TABLE actions on partitioned tables.
This centralizes relation-kind checks in the common permission framework
and produces the standard, consistent error message.
While doing this, fix two related issues in the preparation logic:
- The header comment of ATPrepAddInherit was a stale copy-paste that
described behavior no longer implemented there.
- NO INHERIT previously didn't use ATPrepAddInherit, causing it to reach
execution-time checks unnecessarily. Consolidate preparation into a
shared helper used by both INHERIT and NO INHERIT, ensuring both are
rejected uniformly and as early as possible.
Author: Chao Li <[email protected]>
Reviewed-by: Zsolt Parragi <[email protected]>
Discussion: https://postgr.es/m/CAEoWx2kggo1N2kDH6OSfXHL_5gKg3DqQ0PdNuL4LH4XSTKJ3-g@mail.gmail.com
---
src/backend/commands/tablecmds.c | 31 ++++++++++++-----------
src/test/regress/expected/alter_table.out | 15 ++++++++---
src/test/regress/sql/alter_table.sql | 8 ++++--
3 files changed, 34 insertions(+), 20 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3e9f62e9d37..379f4d4ebaf 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -686,7 +686,7 @@ static void ATExecEnableDisableTrigger(Relation rel, const char *trigname,
LOCKMODE lockmode);
static void ATExecEnableDisableRule(Relation rel, const char *rulename,
char fires_when, LOCKMODE lockmode);
-static void ATPrepAddInherit(Relation child_rel);
+static void ATPrepChangeInherit(Relation child_rel);
static ObjectAddress ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode);
static ObjectAddress ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode);
static void drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid,
@@ -5194,16 +5194,16 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_AddInherit: /* INHERIT */
ATSimplePermissions(cmd->subtype, rel,
- ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_FOREIGN_TABLE);
+ ATT_TABLE | ATT_FOREIGN_TABLE);
/* This command never recurses */
- ATPrepAddInherit(rel);
+ ATPrepChangeInherit(rel);
pass = AT_PASS_MISC;
break;
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 */
- /* No command-specific prep needed */
+ ATPrepChangeInherit(rel);
pass = AT_PASS_MISC;
break;
case AT_AlterConstraint: /* ALTER CONSTRAINT */
@@ -17259,14 +17259,14 @@ ATExecEnableDisableRule(Relation rel, const char *rulename,
}
/*
- * ALTER TABLE INHERIT
+ * Prepare to change inheritance.
*
- * 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.
+ * A child table cannot be a typed table or a partition. We don't need to check
+ * partitioned table here, because it has been blocked by ATSimplePermissions in
+ * ATPrepCmd.
*/
static void
-ATPrepAddInherit(Relation child_rel)
+ATPrepChangeInherit(Relation child_rel)
{
if (child_rel->rd_rel->reloftype)
ereport(ERROR,
@@ -17277,14 +17277,15 @@ ATPrepAddInherit(Relation child_rel)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot change inheritance of a partition")));
-
- if (child_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot change inheritance of partitioned table")));
}
/*
+ * 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.
*/
static ObjectAddress
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index ac1a7345d0f..27b3518f94c 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3997,10 +3997,19 @@ CREATE TABLE nonpartitioned (
a int,
b int
);
-ALTER TABLE partitioned INHERIT nonpartitioned;
-ERROR: cannot change inheritance of partitioned table
-ALTER TABLE nonpartitioned INHERIT partitioned;
+ALTER TABLE partitioned INHERIT nonpartitioned; -- fail
+ERROR: ALTER action INHERIT cannot be performed on relation "partitioned"
+DETAIL: This operation is not supported for partitioned tables.
+ALTER TABLE partitioned NO INHERIT nonpartitioned; -- fail
+ERROR: ALTER action NO INHERIT cannot be performed on relation "partitioned"
+DETAIL: This operation is not supported for partitioned tables.
+ALTER TABLE nonpartitioned INHERIT partitioned; -- fail
ERROR: cannot inherit from partitioned table "partitioned"
+CREATE TABLE partitioned_p1 PARTITION OF partitioned FOR VALUES FROM (0, 0) TO (10, 100);
+ALTER TABLE partitioned_p1 INHERIT nonpartitioned; -- fail
+ERROR: cannot change inheritance of a partition
+ALTER TABLE partitioned_p1 NO INHERIT nonpartitioned; -- fail
+ERROR: cannot change inheritance of a partition
-- cannot add NO INHERIT constraint to partitioned tables
ALTER TABLE partitioned ADD CONSTRAINT chk_a CHECK (a > 0) NO INHERIT;
ERROR: cannot add NO INHERIT constraint to partitioned table "partitioned"
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 417202430a5..24936ecdd34 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2397,8 +2397,12 @@ CREATE TABLE nonpartitioned (
a int,
b int
);
-ALTER TABLE partitioned INHERIT nonpartitioned;
-ALTER TABLE nonpartitioned INHERIT partitioned;
+ALTER TABLE partitioned INHERIT nonpartitioned; -- fail
+ALTER TABLE partitioned NO INHERIT nonpartitioned; -- fail
+ALTER TABLE nonpartitioned INHERIT partitioned; -- fail
+CREATE TABLE partitioned_p1 PARTITION OF partitioned FOR VALUES FROM (0, 0) TO (10, 100);
+ALTER TABLE partitioned_p1 INHERIT nonpartitioned; -- fail
+ALTER TABLE partitioned_p1 NO INHERIT nonpartitioned; -- fail
-- cannot add NO INHERIT constraint to partitioned tables
ALTER TABLE partitioned ADD CONSTRAINT chk_a CHECK (a > 0) NO INHERIT;
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
2026-01-22 09:01 Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-22 23:30 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 00:54 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 07:43 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
2026-01-26 07:10 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-26 17:26 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
2026-01-26 23:13 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
@ 2026-01-27 07:48 ` Michael Paquier <[email protected]>
2026-01-27 08:41 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
2026-01-28 01:56 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
0 siblings, 2 replies; 11+ messages in thread
From: Michael Paquier @ 2026-01-27 07:48 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: Zsolt Parragi <[email protected]>; Postgres hackers <[email protected]>
On Tue, Jan 27, 2026 at 07:13:04AM +0800, Chao Li wrote:
> 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?
There is more to this set of changes than it looks at first sight.
Hence, one question about 0001: can the previous error path in
mark_index_clustered() be reached through a different mean than ALTER
TABLE? If yes, we should have a test for it. If no, it could be
switched to an elog(ERROR) or an assertion. The code paths leading to
the previous error should be analyzed further.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
2026-01-22 09:01 Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-22 23:30 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 00:54 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 07:43 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
2026-01-26 07:10 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-26 17:26 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
2026-01-26 23:13 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-27 07:48 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Michael Paquier <[email protected]>
@ 2026-01-27 08:41 ` Zsolt Parragi <[email protected]>
1 sibling, 0 replies; 11+ messages in thread
From: Zsolt Parragi @ 2026-01-27 08:41 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Chao Li <[email protected]>; Postgres hackers <[email protected]>
> can the previous error path in
> mark_index_clustered() be reached through a different mean than ALTER
> TABLE?
That was one of the things we discussed in the previous emails. The
remaining callers of that function are VACUUM FULL and CLUSTER. It
definitely can't be hit by VACUUM (it passes an InvalidOid for an
index), but CLUSTER is a bit more difficult to follow. It seems like
to me that it shouldn't be hit (CLUSTER only calls it for leaf
partitions, where the check will be false), but I'm not 100% sure
about my diagnosis.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: tablecmds: reject CLUSTER ON for partitioned tables earlier
2026-01-22 09:01 Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-22 23:30 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 00:54 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-23 07:43 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
2026-01-26 07:10 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-26 17:26 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Zsolt Parragi <[email protected]>
2026-01-26 23:13 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-27 07:48 ` Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Michael Paquier <[email protected]>
@ 2026-01-28 01:56 ` Chao Li <[email protected]>
1 sibling, 0 replies; 11+ messages in thread
From: Chao Li @ 2026-01-28 01:56 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; Zsolt Parragi <[email protected]>; PostgreSQL Hackers <[email protected]>
> On Jan 27, 2026, at 15:48, Michael Paquier <[email protected]> wrote:
>
> On Tue, Jan 27, 2026 at 07:13:04AM +0800, Chao Li wrote:
>> 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?
>
> There is more to this set of changes than it looks at first sight.
>
> Hence, one question about 0001: can the previous error path in
> mark_index_clustered() be reached through a different mean than ALTER
> TABLE? If yes, we should have a test for it. If no, it could be
> switched to an elog(ERROR) or an assertion. The code paths leading to
> the previous error should be analyzed further.
Okay, I spent time today investigating mark_index_clustered() today.
First, I reset the source tree to 05fb5d6 where the partitioned table check was added to mark_index_clustered(). The commit subject is "Ignore partitioned indexes where appropriate”. It added the check in 3 functions:
* cluster()
* mark_index_clustered()
* get_relation_info() - not in scope of this discussion
At this commit, ALTER TABLE … CLUSTER ON / SET WITHOUT CLUSTER code patch could reach mark_index_clustered(). Other than that, mark_index_clustered() was only called by rebuild_relation() when the parameter indexOid is valid; rebuild_relation() was only called by cluster_rel(); and cluster_rel() was called by vacuum_rel() and cluster().
* For the cluster() code patch: because of the check added to cluster() by this commit, partitioned table would return early, thus mark_index_clustered() was actually not called.
* For the vacuum_rel() code path: there was already a check for partitioned table to return early, thus cluster_rel() won’t be called against partitioned tables, so that mark_index_clustered() could not be called either.
So, looks like the check added to mark_index_clustered() was only for the ALTER TABLE code path.
Then, switching the source tree back to this patch. Now, for the ALTER TABLE code path, 0001 ensures partitioned table won’t reach mark_index_clustered().
Other than ALTER TABLE, mark_index_clustered() is only called by rebuild_relation(); rebuild_relation() is only called by cluster_rel(); cluster_rel() is called by vacuum_rel() and cluster(). So, the call paths are the same as commit 05fb5d6.
For the cluster() code path, I traced this scenario:
```
evantest=# create table p (id int) partition by range (id);
CREATE TABLE
evantest=# create table p1 partition of p for values from (0) to (10);
CREATE TABLE
evantest=# create index p_idx on p (id);
CREATE INDEX
evantest=# cluster p using p_idx;
CLUSTER
```
The code ran into cluster(). Now, cluster() is much complicated than it was in commit 05fb5d6. For a partitioned table, it iterates all leaf partitions to call cluster_rel():
```
/*
* Either we're processing a partitioned table, or we were not given any
* table name at all. In either case, obtain a list of relations to
* process.
*
* In the former case, an index name must have been given, so we don't
* need to recheck its "indisclustered" bit, but we have to check that it
* is an index that we can cluster on. In the latter case, we set the
* option bit to have indisclustered verified.
*
* Rechecking the relation itself is necessary here in all cases.
*/
params.options |= CLUOPT_RECHECK;
if (rel != NULL)
{
Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
check_index_is_clusterable(rel, indexOid, AccessShareLock);
rtcs = get_tables_to_cluster_partitioned(cluster_context, indexOid);
/* close relation, releasing lock on parent table */
table_close(rel, AccessExclusiveLock);
}
….
cluster_multiple_rels(rtcs, ¶ms); // cluster_rel() is called here
```
So, partitioned table should never reach mark_index_clustered() from the cluster() code patch.
For the vacuum_rel() code patch, same as before, partitioned table will return early, cluster_rel() won’t be called at all:
```
/*
* Silently ignore partitioned tables as there is no work to be done. The
* useful work is on their child partitions, which have been queued up for
* us separately.
*/
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
{
relation_close(rel, lmode);
PopActiveSnapshot();
CommitTransactionCommand();
/* It's OK to proceed with ANALYZE on this table */
return true;
}
```
In summary, with 0001, the partitioned table check in mark_index_clustered() is no longer needed. But as mark_index_clustered() is an extern-ed function, it might have future callers, I think we can change ereport(ERROR) to an Assert(). I will include the change in next revision.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2026-01-28 01:56 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-01-22 09:01 Re: tablecmds: reject CLUSTER ON for partitioned tables earlier Chao Li <[email protected]>
2026-01-22 23:30 ` Chao Li <[email protected]>
2026-01-23 00:54 ` Chao Li <[email protected]>
2026-01-23 07:43 ` Zsolt Parragi <[email protected]>
2026-01-23 07:53 ` Chao Li <[email protected]>
2026-01-26 07:10 ` Chao Li <[email protected]>
2026-01-26 17:26 ` Zsolt Parragi <[email protected]>
2026-01-26 23:13 ` Chao Li <[email protected]>
2026-01-27 07:48 ` Michael Paquier <[email protected]>
2026-01-27 08:41 ` Zsolt Parragi <[email protected]>
2026-01-28 01:56 ` Chao Li <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox