On Mon, Dec 20, 2021, at 8:11 AM, Michael Paquier wrote:
> On Mon, Dec 20, 2021 at 03:46:13AM +0000, wangw.f...@fujitsu.com wrote:
> > Here is a patch to correct wrong comment about
> > REPLICA_IDENTITY_INDEX, And improve the pg-doc.
>
> That's mostly fine. I have made some adjustments as per the
> attached.
Your patch looks good to me.
> Pondering more about this thread, I don't think we should change the
> existing behavior in the back-branches, but I don't have any arguments
> about doing such changes on HEAD to help the features being worked
> on, either. So I'd like to apply and back-patch the attached, as a
> first step, to fix the inconsistency.
>
What do you think about the attached patch? It forbids the DROP INDEX. We might
add a detail message but I didn't in this patch.
--
Euler Taveira
EDB https://www.enterprisedb.com/
From 2ba00335746cf8348e019582a253721047f1a0ac Mon Sep 17 00:00:00 2001
From: Euler Taveira <euler.tave...@enterprisedb.com>
Date: Thu, 16 Dec 2021 16:54:47 -0300
Subject: [PATCH v1] Disallow dropping an index that is used by replica
identity
Drop an index that is used by a replica identity can break the
replication for UPDATE and DELETE operations. This may be undesirable
from the usability standpoint. It introduces a behaviour change.
---
src/backend/commands/tablecmds.c | 14 +++++++++++++-
src/test/regress/expected/replica_identity.out | 3 +++
src/test/regress/sql/replica_identity.sql | 3 +++
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index bf42587e38..727ad6625b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1530,15 +1530,21 @@ RangeVarCallbackForDropRelation(const RangeVar *rel, Oid relOid, Oid oldRelOid,
rel->relname);
/*
+ * An index used by a replica identity cannot be dropped because some
+ * operations (UPDATE and DELETE) relies on the index properties
+ * (uniqueness and NOT NULL) to record old values. These old values are
+ * essential for identifying the row on the subscriber.
+ *
* Check the case of a system index that might have been invalidated by a
* failed concurrent process and allow its drop. For the time being, this
* only concerns indexes of toast relations that became invalid during a
* REINDEX CONCURRENTLY process.
*/
- if (IsSystemClass(relOid, classform) && relkind == RELKIND_INDEX)
+ if (relkind == RELKIND_INDEX)
{
HeapTuple locTuple;
Form_pg_index indexform;
+ bool indisreplident;
bool indisvalid;
locTuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(relOid));
@@ -1550,8 +1556,14 @@ RangeVarCallbackForDropRelation(const RangeVar *rel, Oid relOid, Oid oldRelOid,
indexform = (Form_pg_index) GETSTRUCT(locTuple);
indisvalid = indexform->indisvalid;
+ indisreplident = indexform->indisreplident;
ReleaseSysCache(locTuple);
+ if (indisreplident)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("could not drop index \"%s\" because it is required by the replica identity", rel->relname)));
+
/* Mark object as being an invalid index of system catalogs */
if (!indisvalid)
invalid_system_index = true;
diff --git a/src/test/regress/expected/replica_identity.out b/src/test/regress/expected/replica_identity.out
index e25ec06a84..56900451f7 100644
--- a/src/test/regress/expected/replica_identity.out
+++ b/src/test/regress/expected/replica_identity.out
@@ -227,6 +227,9 @@ Indexes:
-- used as replica identity.
ALTER TABLE test_replica_identity3 ALTER COLUMN id DROP NOT NULL;
ERROR: column "id" is in index used as replica identity
+-- DROP INDEX is not allowed if the index is used as REPLICA IDENTITY
+DROP INDEX test_replica_identity3_id_key;
+ERROR: could not drop index "test_replica_identity3_id_key" because it is required by the replica identity
DROP TABLE test_replica_identity;
DROP TABLE test_replica_identity2;
DROP TABLE test_replica_identity3;
diff --git a/src/test/regress/sql/replica_identity.sql b/src/test/regress/sql/replica_identity.sql
index 33da829713..f46769e965 100644
--- a/src/test/regress/sql/replica_identity.sql
+++ b/src/test/regress/sql/replica_identity.sql
@@ -98,6 +98,9 @@ ALTER TABLE test_replica_identity3 ALTER COLUMN id TYPE bigint;
-- used as replica identity.
ALTER TABLE test_replica_identity3 ALTER COLUMN id DROP NOT NULL;
+-- DROP INDEX is not allowed if the index is used as REPLICA IDENTITY
+DROP INDEX test_replica_identity3_id_key;
+
DROP TABLE test_replica_identity;
DROP TABLE test_replica_identity2;
DROP TABLE test_replica_identity3;
--
2.20.1
