Feedback-ID: ie3de48e3:Fastmail
Date: Thu, 10 Sep 2026 12:22:52 +0200
From: Alvaro Herrera <alvherre@kurilemu.de>
To: Antonin Houska <ah@cybertec.at>
Cc: Chao Li <li.evan.chao@gmail.com>,
	Matthias van de Meent <boekewurm+postgres@gmail.com>,
 Nathan Bossart <nathandbossart@gmail.com>,
	pgsql-hackers@postgresql.org
Subject: Re: REPACK (CONCURRENTLY) fails when replica identity index is
 dropped
Message-ID: <aqKC6K_QO0OEv3gy@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="csvqstbdygoam7e5"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <apcP281A4ta2FToT@alvherre.pgsql>
Archived-At: 
 <https://www.postgresql.org/message-id/aqKC6K_QO0OEv3gy%40alvherre.pgsql>
Precedence: bulk


--csvqstbdygoam7e5
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2026-Sep-01, Alvaro Herrera wrote:

> On 2026-Sep-01, Antonin Houska wrote:
> 
> > I agree that the core issue is that we allow dropping an index that is being
> > used as replica identity.
> >
> > Regarding catalog entries already broken this way, it appears that pg_upgrade
> > fixes them because pg_dump does not issue "ALTER TABLE ... REPLICA IDENTITY
> > USING INDEX ..." if there is not identity index. Thus after pg_restore,
> > pg_class(relreplident) becomes REPLICA_IDENTITY_DEFAULT.
> 
> I agree that disallowing the drop is a sensible thing to do.

Actually, wouldn't it make more sense to reset the replica identity back
to 'd' when the index is dropped, as in the attached patch?

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"After a quick R of TFM, all I can say is HOLY CR** THAT IS COOL! PostgreSQL was
amazing when I first started using it at 7.2, and I'm continually astounded by
learning new features and techniques made available by the continuing work of
the development team."
Berend Tober, http://archives.postgresql.org/pgsql-hackers/2007-08/msg01009.php

--csvqstbdygoam7e5
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename=0001-Revert-replica-identity-to-default-if-the-index-is-d.patch

From 52d58f63db9e0e6cde0729470f41d1f25d972544 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Thu, 10 Sep 2026 12:21:16 +0200
Subject: [PATCH] Revert replica identity to 'default' if the index is dropped

---
 contrib/test_decoding/expected/ddl.out |  6 +++---
 src/backend/catalog/index.c            | 12 ++++++++++++
 src/backend/commands/tablecmds.c       |  2 +-
 src/include/commands/tablecmds.h       |  4 ++++
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/contrib/test_decoding/expected/ddl.out b/contrib/test_decoding/expected/ddl.out
index a129c016d2b..b403d9e06cb 100644
--- a/contrib/test_decoding/expected/ddl.out
+++ b/contrib/test_decoding/expected/ddl.out
@@ -828,7 +828,7 @@ table public.table_dropped_index_with_pk: INSERT: a[integer]:2 b[integer]:2 c[in
 table public.table_dropped_index_with_pk: INSERT: a[integer]:3 b[integer]:3 c[integer]:3
 COMMIT
 BEGIN
-table public.table_dropped_index_with_pk: UPDATE: a[integer]:4 b[integer]:1 c[integer]:1
+table public.table_dropped_index_with_pk: UPDATE: old-key: a[integer]:1 new-tuple: a[integer]:4 b[integer]:1 c[integer]:1
 COMMIT
 BEGIN
 table public.table_dropped_index_with_pk: UPDATE: a[integer]:2 b[integer]:5 c[integer]:2
@@ -837,10 +837,10 @@ BEGIN
 table public.table_dropped_index_with_pk: UPDATE: a[integer]:3 b[integer]:6 c[integer]:7
 COMMIT
 BEGIN
-table public.table_dropped_index_with_pk: DELETE: (no-tuple-data)
+table public.table_dropped_index_with_pk: DELETE: a[integer]:4
 COMMIT
 BEGIN
-table public.table_dropped_index_with_pk: DELETE: (no-tuple-data)
+table public.table_dropped_index_with_pk: DELETE: a[integer]:3
 COMMIT
 BEGIN
 table public.table_dropped_index_no_pk: INSERT: a[integer]:1 b[integer]:1 c[integer]:1
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index ec21b83b6b8..144a7a6acc7 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2352,6 +2352,18 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode)
 		TransferPredicateLocksToHeapRelation(userIndexRelation);
 	}
 
+	/*
+	 * If this index is the replica identity of its table, mark the table as
+	 * having default replica identity.
+	 */
+	if (userHeapRelation->rd_rel->relreplident == REPLICA_IDENTITY_INDEX &&
+		RelationGetReplicaIndex(userHeapRelation) == indexId)
+	{
+		relation_mark_replica_identity(userHeapRelation, REPLICA_IDENTITY_DEFAULT,
+									   InvalidOid, true);
+		CommandCounterIncrement();
+	}
+
 	/*
 	 * Schedule physical removal of the files (if any)
 	 */
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8dc70bfa0f1..1040240f560 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -19065,7 +19065,7 @@ ATExecDropOf(Relation rel, LOCKMODE lockmode)
  * Caller had better hold an exclusive lock on the relation, as the results
  * of running two of these concurrently wouldn't be pretty.
  */
-static void
+void	/* XXX removal of static not for commit */
 relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid,
 							   bool is_internal)
 {
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index c3d8518cb62..22b5472d19c 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -45,6 +45,10 @@ extern void AlterTableInternal(Oid relid, List *cmds, bool recurse);
 
 extern Oid	AlterTableMoveAll(AlterTableMoveAllStmt *stmt);
 
+/* XXX not for commit */
+extern void relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid,
+										   bool is_internal);
+
 extern ObjectAddress AlterTableNamespace(AlterObjectSchemaStmt *stmt,
 										 Oid *oldschema);
 
-- 
2.47.3


--csvqstbdygoam7e5--