public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: David G. Johnston <[email protected]>
To: Greg Sabino Mullane <[email protected]>
Cc: Postgres hackers <[email protected]>
Subject: Re: ALTER TABLE: warn when actions do not recurse to partitions
Date: Tue, 13 Jan 2026 11:18:58 +0800
Message-ID: <CAEoWx2nic2MaPUEKwb0Me1iwO3LsCf8wUHm55pjqaTRgJEZpcQ@mail.gmail.com> (raw)
In-Reply-To: <CAKFQuwZXt2dAuHFs6MtRCzfMP6YZFzssuzdW2woJBVmco=CDdQ@mail.gmail.com>
References: <CAEoWx2=SLga-xH09Cq_PAvsHhQHrBK+V0vF821JKgzS=Bm0haA@mail.gmail.com>
<CAKFQuwZXt2dAuHFs6MtRCzfMP6YZFzssuzdW2woJBVmco=CDdQ@mail.gmail.com>
Hi David and Greg,
Thanks a lot for your reviews and feedbacks.
On Jan 12, 2026, at 22:40, David G. Johnston <[email protected]>
wrote:
On Monday, January 12, 2026, Chao Li <[email protected]> wrote:
For now, I’ve limited the change to REPLICA IDENTITY to see whether there
are objections to this approach. If there are none, I plan to extend the
same warning behavior to the other sub-commands listed above. After that,
users can reasonably assume that an ALTER TABLE partitioned_table
... action will recurse to child partitions unless a warning explicitly
tells them otherwise.
It should be a notice, not a warning.
Make sense. I changed to NOTICE in v2.
How about indicating how many partitions were affected in the notice and
I added partition count in the notice message in v2.
allowing the absence of such a notice to be the indicator that cascading
did not happen?
David J.
I don’t think relying on the absence of a notice works well in this case.
Currently, cascading never happens, which is exactly why I’m adding a
NOTICE. If we rely on silence to indicate “no cascade”, users have no
signal that their expectation was incorrect.
The ALTER TABLE documentation says:
```
If ONLY is specified before the table name, only that table is altered.
If ONLY is not specified, the table and all its descendant tables (if any)
are altered.
```
Given this, users reasonably expect that omitting ONLY will cause REPLICA
IDENTITY to cascade to partitions. In reality, it never does, which breaks
that expectation. The NOTICE is intended to make this behavior explicit in
exactly that case.
On Jan 12, 2026, at 23:23, Greg Sabino Mullane <[email protected]> wrote:
On Mon, Jan 12, 2026 at 9:40 AM David G. Johnston <
[email protected]> wrote:
How about indicating how many partitions were affected in the notice and
allowing the absence of such a notice to be the indicator that cascading
did not happen?
I like the idea of number of partitions, but think we need to be more
explicit than people surmising the lack of a notice is significant.
Cheers,
Greg
As explained above, the NOTICE is only emitted in the case where the
documented ALTER TABLE semantics suggest cascading, but REPLICA IDENTITY
does not actually cascade to partitions. This makes the behavior explicit,
rather than relying on users to infer meaning from the absence of a message.
PSA v2:
* Changed level log to NOTICE
* Rephrased the notice message and included partition count in the message.
Now, the message is like:
```
evantest=# alter table sensor_data replica identity full;
NOTICE: REPLICA IDENTITY does not apply to partitions (1 affected)
ALTER TABLE
```
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v2-0001-Add-notice-when-ALTER-TABLE-REPLICA-IDENTITY-does.patch (3.6K, 3-v2-0001-Add-notice-when-ALTER-TABLE-REPLICA-IDENTITY-does.patch)
download | inline diff:
From 2fb2578efc74dbf9027ca0f65e4171ad4d6d964c Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Mon, 12 Jan 2026 16:56:58 +0800
Subject: [PATCH v2] Add notice when ALTER TABLE REPLICA IDENTITY does not
recurse
ALTER TABLE ... REPLICA IDENTITY accepts a recursive form on
partitioned tables, but the change is applied only to the partitioned
table itself and does not propagate to child partitions.
Previously this case was silently accepted, which could mislead users
into assuming that the setting would recurse. Add a notice when
recursion is requested on a partitioned table to make the behavior
explicit and avoid confusion.
This change does not alter semantics; it only provides user-visible
feedback. Similar notices may be added for other ALTER TABLE
sub-commands with non-recursive behavior in follow-up commits.
Author: Chao Li <[email protected]>
---
src/backend/commands/tablecmds.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f976c0e5c7e..2e8d769065c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -693,7 +693,7 @@ static void drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid,
DependencyType deptype);
static ObjectAddress ATExecAddOf(Relation rel, const TypeName *ofTypename, LOCKMODE lockmode);
static void ATExecDropOf(Relation rel, LOCKMODE lockmode);
-static void ATExecReplicaIdentity(Relation rel, ReplicaIdentityStmt *stmt, LOCKMODE lockmode);
+static void ATExecReplicaIdentity(Relation rel, ReplicaIdentityStmt *stmt, bool recurse, LOCKMODE lockmode);
static void ATExecGenericOptions(Relation rel, List *options);
static void ATExecSetRowSecurity(Relation rel, bool rls);
static void ATExecForceNoForceRowSecurity(Relation rel, bool force_rls);
@@ -5227,8 +5227,15 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
ATSimplePermissions(cmd->subtype, rel,
ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_MATVIEW);
pass = AT_PASS_MISC;
- /* This command never recurses */
- /* No command-specific prep needed */
+
+ /*
+ * This command now doesn't recurse, but we want to notify user if
+ * recurse is set
+ *
+ * No command-specific prep needed
+ */
+ if (recurse)
+ cmd->recurse = true;
break;
case AT_EnableTrig: /* ENABLE TRIGGER variants */
case AT_EnableAlwaysTrig:
@@ -5643,7 +5650,8 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
ATExecDropOf(rel, lockmode);
break;
case AT_ReplicaIdentity:
- ATExecReplicaIdentity(rel, (ReplicaIdentityStmt *) cmd->def, lockmode);
+ ATExecReplicaIdentity(rel, (ReplicaIdentityStmt *) cmd->def,
+ cmd->recurse, lockmode);
break;
case AT_EnableRowSecurity:
ATExecSetRowSecurity(rel, true);
@@ -18515,12 +18523,22 @@ relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid,
* ALTER TABLE <name> REPLICA IDENTITY ...
*/
static void
-ATExecReplicaIdentity(Relation rel, ReplicaIdentityStmt *stmt, LOCKMODE lockmode)
+ATExecReplicaIdentity(Relation rel, ReplicaIdentityStmt *stmt, bool recurse, LOCKMODE lockmode)
{
Oid indexOid;
Relation indexRel;
int key;
+ if (recurse && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ PartitionDesc pd = RelationGetPartitionDesc(rel, true);
+ int nparts = pd->nparts;
+
+ ereport(NOTICE,
+ (errmsg("REPLICA IDENTITY does not apply to partitions (%d affected)",
+ nparts)));
+ }
+
if (stmt->identity_type == REPLICA_IDENTITY_DEFAULT)
{
relation_mark_replica_identity(rel, stmt->identity_type, InvalidOid, true);
--
2.39.5 (Apple Git-154)
view thread (5+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: ALTER TABLE: warn when actions do not recurse to partitions
In-Reply-To: <CAEoWx2nic2MaPUEKwb0Me1iwO3LsCf8wUHm55pjqaTRgJEZpcQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox