REINDEX CONCURRENTLY and indisreplident

Started by Michael Paquierover 5 years ago4 messages
#1Michael Paquier
michael@paquier.xyz
1 attachment(s)

Hi all,

I have bumped into $subject, causing a replica identity index to
be considered as dropped if running REINDEX CONCURRENTLY on it. This
means that the old tuple information would get lost in this case, as
a REPLICA IDENTITY USING INDEX without a dropped index is the same as
NOTHING.

Attached is a fix for this issue, that needs a backpatch down to 12.
Thanks,
--
Michael

Attachments:

reindex-conc-replident.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 7cfbdd57db..cdc01c49c9 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1538,6 +1538,10 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, const char *oldName)
 	newIndexForm->indimmediate = oldIndexForm->indimmediate;
 	oldIndexForm->indimmediate = true;
 
+	/* Preserve indisreplident in the new index */
+	newIndexForm->indisreplident = oldIndexForm->indisreplident;
+	oldIndexForm->indisreplident = false;
+
 	/* Preserve indisclustered in the new index */
 	newIndexForm->indisclustered = oldIndexForm->indisclustered;
 
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index ae95bb38a6..e3e6634d7e 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2141,6 +2141,27 @@ SELECT indexrelid::regclass, indisclustered FROM pg_index
 (1 row)
 
 DROP TABLE concur_clustered;
+-- Check that indisreplident updates are preserved.
+CREATE TABLE concur_replident(i int NOT NULL);
+CREATE UNIQUE INDEX concur_replident_i_idx ON concur_replident(i);
+ALTER TABLE concur_replident REPLICA IDENTITY
+  USING INDEX concur_replident_i_idx;
+SELECT indexrelid::regclass, indisreplident FROM pg_index
+  WHERE indrelid = 'concur_replident'::regclass;
+       indexrelid       | indisreplident 
+------------------------+----------------
+ concur_replident_i_idx | t
+(1 row)
+
+REINDEX TABLE CONCURRENTLY concur_replident;
+SELECT indexrelid::regclass, indisreplident FROM pg_index
+  WHERE indrelid = 'concur_replident'::regclass;
+       indexrelid       | indisreplident 
+------------------------+----------------
+ concur_replident_i_idx | t
+(1 row)
+
+DROP TABLE concur_replident;
 -- Partitions
 -- Create some partitioned tables
 CREATE TABLE concur_reindex_part (c1 int, c2 int) PARTITION BY RANGE (c1);
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index c3246cb296..f3667bacdc 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -866,6 +866,17 @@ REINDEX TABLE CONCURRENTLY concur_clustered;
 SELECT indexrelid::regclass, indisclustered FROM pg_index
   WHERE indrelid = 'concur_clustered'::regclass;
 DROP TABLE concur_clustered;
+-- Check that indisreplident updates are preserved.
+CREATE TABLE concur_replident(i int NOT NULL);
+CREATE UNIQUE INDEX concur_replident_i_idx ON concur_replident(i);
+ALTER TABLE concur_replident REPLICA IDENTITY
+  USING INDEX concur_replident_i_idx;
+SELECT indexrelid::regclass, indisreplident FROM pg_index
+  WHERE indrelid = 'concur_replident'::regclass;
+REINDEX TABLE CONCURRENTLY concur_replident;
+SELECT indexrelid::regclass, indisreplident FROM pg_index
+  WHERE indrelid = 'concur_replident'::regclass;
+DROP TABLE concur_replident;
 
 -- Partitions
 -- Create some partitioned tables
#2Euler Taveira
euler.taveira@2ndquadrant.com
In reply to: Michael Paquier (#1)
Re: REINDEX CONCURRENTLY and indisreplident

On Wed, 3 Jun 2020 at 03:54, Michael Paquier <michael@paquier.xyz> wrote:

Hi all,

I have bumped into $subject, causing a replica identity index to
be considered as dropped if running REINDEX CONCURRENTLY on it. This
means that the old tuple information would get lost in this case, as
a REPLICA IDENTITY USING INDEX without a dropped index is the same as
NOTHING.

LGTM. I tested in both versions (12, master) and it works accordingly.

--
Euler Taveira http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#3Michael Paquier
michael@paquier.xyz
In reply to: Euler Taveira (#2)
Re: REINDEX CONCURRENTLY and indisreplident

On Wed, Jun 03, 2020 at 12:40:38PM -0300, Euler Taveira wrote:

On Wed, 3 Jun 2020 at 03:54, Michael Paquier <michael@paquier.xyz> wrote:

I have bumped into $subject, causing a replica identity index to
be considered as dropped if running REINDEX CONCURRENTLY on it. This
means that the old tuple information would get lost in this case, as
a REPLICA IDENTITY USING INDEX without a dropped index is the same as
NOTHING.

LGTM. I tested in both versions (12, master) and it works accordingly.

Thanks for the review. I'll try to get that fixed soon.

By the way, your previous email was showing up as part of my own email
with the indentation that was used so I missed it first. That's the
case as well here:
/messages/by-id/CAH503wDaejzhP7+wA-hHS6c7NzE69oWqe5Zf_TYFu1epAwp6EQ@mail.gmail.com
--
Michael

#4Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#3)
Re: REINDEX CONCURRENTLY and indisreplident

On Thu, Jun 04, 2020 at 11:23:36AM +0900, Michael Paquier wrote:

On Wed, Jun 03, 2020 at 12:40:38PM -0300, Euler Taveira wrote:

On Wed, 3 Jun 2020 at 03:54, Michael Paquier <michael@paquier.xyz> wrote:

I have bumped into $subject, causing a replica identity index to
be considered as dropped if running REINDEX CONCURRENTLY on it. This
means that the old tuple information would get lost in this case, as
a REPLICA IDENTITY USING INDEX without a dropped index is the same as
NOTHING.

LGTM. I tested in both versions (12, master) and it works accordingly.

Thanks for the review. I'll try to get that fixed soon.

Applied this one, just in time before the branching:
/messages/by-id/1931934b-09dc-e93e-fab9-78c5bc72743d@postgresql.org
--
Michael