public inbox for [email protected]
help / color / mirror / Atom feedFrom: Euler Taveira <[email protected]>
To: Michael Paquier <[email protected]>
To: [email protected] <[email protected]>
Cc: Ashutosh Bapat <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Confused comment about drop replica identity index
Date: Mon, 20 Dec 2021 11:57:32 -0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <OS3PR01MB6275464AD0A681A0793F56879E759@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<CAExHW5uWwcV0-=JQXYhEpnEtmwmZQsh0c1N3cxxBYRhwE9WuvQ@mail.gmail.com>
<Yblf/[email protected]>
<OS3PR01MB6275A10D03572E27C4E2C9769E769@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<[email protected]>
<OS3PR01MB62750A124D384B4E96CA43F19E779@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<OS3PR01MB62750A0A1A282E9E0148172B9E7B9@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<[email protected]>
On Mon, Dec 20, 2021, at 8:11 AM, Michael Paquier wrote:
> On Mon, Dec 20, 2021 at 03:46:13AM +0000, [email protected] 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/
Attachments:
[text/x-patch] v1-0001-Disallow-dropping-an-index-that-is-used-by-replic.patch (3.7K, ../[email protected]/3-v1-0001-Disallow-dropping-an-index-that-is-used-by-replic.patch)
download | inline diff:
From 2ba00335746cf8348e019582a253721047f1a0ac Mon Sep 17 00:00:00 2001
From: Euler Taveira <[email protected]>
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
view thread (17+ 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], [email protected]
Subject: Re: Confused comment about drop replica identity index
In-Reply-To: <[email protected]>
* 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