agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedREPACK (CONCURRENTLY) fails when replica identity index is dropped
12+ messages / 6 participants
[nested] [flat]
* REPACK (CONCURRENTLY) fails when replica identity index is dropped
@ 2026-08-27 18:26 Nathan Bossart <nathandbossart@gmail.com>
2026-08-27 19:31 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Matthias van de Meent <boekewurm+postgres@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Nathan Bossart @ 2026-08-27 18:26 UTC (permalink / raw)
To: pgsql-hackers; +Cc: alvherre@kurilemu.de
I don't fully understand the mechanics of this one, but here is a
reproducer:
CREATE TABLE t (a INT PRIMARY KEY, b INT, c TEXT);
INSERT INTO t SELECT g, g, repeat('x', 1000) FROM generate_series(1, 1000000) g;
CREATE UNIQUE INDEX i ON t (a);
ALTER TABLE t REPLICA IDENTITY USING INDEX i;
DROP INDEX i;
REPACK (CONCURRENTLY) t;
-- in a separate session, while REPACK is still running
DELETE FROM t WHERE a = 1;
This produces the following ERROR from the REPACK command:
ERROR: incomplete delete info
CONTEXT: slot "pg_repack_34213", output plugin "pgrepack", in the change callback, associated LSN 0/A6CC1ED0
REPACK decoding worker
I think this can be addressed by verifying the index exists in
check_concurrent_repack_requirements() and erroring out if it doesn't.
--
nathan
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped
2026-08-27 18:26 REPACK (CONCURRENTLY) fails when replica identity index is dropped Nathan Bossart <nathandbossart@gmail.com>
@ 2026-08-27 19:31 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
2026-08-28 06:04 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Chao Li <li.evan.chao@gmail.com>
2026-08-28 08:29 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Ewan Young <kdbase.hack@gmail.com>
0 siblings, 2 replies; 12+ messages in thread
From: Matthias van de Meent @ 2026-08-27 19:31 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers; alvherre@kurilemu.de
On Thu, 27 Aug 2026 at 20:26, Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> I don't fully understand the mechanics of this one, but here is a
> reproducer:
>
> CREATE TABLE t (a INT PRIMARY KEY, b INT, c TEXT);
> INSERT INTO t SELECT g, g, repeat('x', 1000) FROM generate_series(1, 1000000) g;
> CREATE UNIQUE INDEX i ON t (a);
> ALTER TABLE t REPLICA IDENTITY USING INDEX i;
> DROP INDEX i;
> REPACK (CONCURRENTLY) t;
>
> -- in a separate session, while REPACK is still running
> DELETE FROM t WHERE a = 1;
>
> This produces the following ERROR from the REPACK command:
>
> ERROR: incomplete delete info
> CONTEXT: slot "pg_repack_34213", output plugin "pgrepack", in the change callback, associated LSN 0/A6CC1ED0
> REPACK decoding worker
>
> I think this can be addressed by verifying the index exists in
> check_concurrent_repack_requirements() and erroring out if it doesn't.
I think the issue is caused by the following: REPACK's check has the
incorrect assumption that REPLICA IDENTITY USING INDEX either reverts
to DEFAULT or falls back to the behaviour of DEFAULT if the identity
index gets dropped, and thus uses GetRelationIdentityOrPK(), which
hides a lack of replica identity index. The issue shows up due to the
following garden path of data flows:
1. A table with REPLICA IDENTITY USING INDEX doesn't fall back to
REPLICA IDENTITY DEFAULT once the identity index is dropped.
2. In the catcache, the table won't fall back to rd_replidindex =
pkeyIndex when replident='i', but instead will set
rd_replidindex=InvalidOid.
See the tail end of RelationGetIndexList.
3. Then, in heap_delete, it calls ExtractReplicaIdentity() to find the
key of the deleted tuple.
3a. ExtractR_I_() checks the identity key attributes from
RelationGetIndexAttrBitmap(..., INDEX_ATTR_BITMAP_IDENTITY_KEY), which
also only uses rd_replidindex, and doesn't fall back to the primary
key index's attributes.
3b. If ExtractR_I_() doesn't have identity key attributes, it returns NULL
3c. heap_delete thus doesn't have any logical identity attributes to
log, and treats the delete operation as any non-logical deletion when
logging the data.
4. Finally, the DELETE record gets decoded, and the logical plugin
finds out that no logical key data was included, and promptly ERRORs
out.
The attached patch is a blind shot that I suspect will fix the issue.
Kind regards,
Matthias van de Meent
Databricks (https://www.databricks.com)
Attachments:
[application/octet-stream] v1-0001-Repack-Fix-replica-identity-index-check.patch (2.8K, ../../CAEze2WhT2=bm8s45MjQf+YJ5Md7TTXy=8MKKOYsWspHGXxNWKA@mail.gmail.com/2-v1-0001-Repack-Fix-replica-identity-index-check.patch)
download | inline diff:
From e22fc4cb778024ce23e538e41966dc3ed2543e01 Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Thu, 27 Aug 2026 21:26:21 +0200
Subject: [PATCH v1] Repack: Fix replica identity index check
We can't fall back to the PK index, because replica identity systems
elsewhere don't do that either when they determine which data to log.
---
src/backend/commands/repack.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 477c86b2ba6..5a05bb41a68 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -924,26 +924,13 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p)
"REPLICA IDENTITY NOTHING" : "REPLICA IDENTITY FULL"));
/*
- * Obtain the replica identity index -- either one that has been set
- * explicitly, or a non-deferrable primary key. If none of these cases
- * apply, the table cannot be repacked concurrently. It might be possible
- * to have repack work with a FULL replica identity; however that requires
- * more work and is not implemented yet.
+ * Obtain the replica identity index. We require the actual index
+ * responsible for being the logical identity; because AMs use that
+ * same index to determine which identity to include in WAL.
*/
- ident_idx = GetRelationIdentityOrPK(rel);
+ ident_idx = RelationGetReplicaIndex(rel);
if (!OidIsValid(ident_idx))
{
- /* This special case warrants its own error message */
- if (OidIsValid(rel->rd_pkindex) && rel->rd_ispkdeferrable)
- ereport(ERROR,
- errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot execute %s on relation \"%s\"",
- "REPACK (CONCURRENTLY)",
- RelationGetRelationName(rel)),
- errdetail("%s does not support deferrable primary keys.",
- "REPACK (CONCURRENTLY)"),
- errhint("Use ALTER TABLE ... REPLICA IDENTITY USING INDEX to designate another index as replica identity."));
-
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("cannot execute %s on relation \"%s\"",
@@ -952,6 +939,19 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p)
RelationGetRelationName(rel)));
}
+ /* This special case warrants its own error message */
+ if (!OidIsValid(RelationGetPrimaryKeyIndex(rel, false)))
+ {
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot execute %s on relation \"%s\"",
+ "REPACK (CONCURRENTLY)",
+ RelationGetRelationName(rel)),
+ errdetail("%s does not support deferrable primary keys.",
+ "REPACK (CONCURRENTLY)"),
+ errhint("Use ALTER TABLE ... REPLICA IDENTITY USING INDEX to designate another index as replica identity."));
+ }
+
*ident_idx_p = ident_idx;
}
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped
2026-08-27 18:26 REPACK (CONCURRENTLY) fails when replica identity index is dropped Nathan Bossart <nathandbossart@gmail.com>
2026-08-27 19:31 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Matthias van de Meent <boekewurm+postgres@gmail.com>
@ 2026-08-28 06:04 ` Chao Li <li.evan.chao@gmail.com>
2026-09-01 10:10 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Antonin Houska <ah@cybertec.at>
1 sibling, 1 reply; 12+ messages in thread
From: Chao Li @ 2026-08-28 06:04 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers; alvherre@kurilemu.de
> On Aug 28, 2026, at 03:31, Matthias van de Meent <boekewurm+postgres@gmail.com> wrote:
>
> On Thu, 27 Aug 2026 at 20:26, Nathan Bossart <nathandbossart@gmail.com> wrote:
>>
>> I don't fully understand the mechanics of this one, but here is a
>> reproducer:
>>
>> CREATE TABLE t (a INT PRIMARY KEY, b INT, c TEXT);
>> INSERT INTO t SELECT g, g, repeat('x', 1000) FROM generate_series(1, 1000000) g;
>> CREATE UNIQUE INDEX i ON t (a);
>> ALTER TABLE t REPLICA IDENTITY USING INDEX i;
>> DROP INDEX i;
>> REPACK (CONCURRENTLY) t;
>>
>> -- in a separate session, while REPACK is still running
>> DELETE FROM t WHERE a = 1;
>>
>> This produces the following ERROR from the REPACK command:
>>
>> ERROR: incomplete delete info
>> CONTEXT: slot "pg_repack_34213", output plugin "pgrepack", in the change callback, associated LSN 0/A6CC1ED0
>> REPACK decoding worker
>>
>> I think this can be addressed by verifying the index exists in
>> check_concurrent_repack_requirements() and erroring out if it doesn't.
>
> I think the issue is caused by the following: REPACK's check has the
> incorrect assumption that REPLICA IDENTITY USING INDEX either reverts
> to DEFAULT or falls back to the behaviour of DEFAULT if the identity
> index gets dropped, and thus uses GetRelationIdentityOrPK(), which
> hides a lack of replica identity index. The issue shows up due to the
> following garden path of data flows:
>
> 1. A table with REPLICA IDENTITY USING INDEX doesn't fall back to
> REPLICA IDENTITY DEFAULT once the identity index is dropped.
> 2. In the catcache, the table won't fall back to rd_replidindex =
> pkeyIndex when replident='i', but instead will set
> rd_replidindex=InvalidOid.
> See the tail end of RelationGetIndexList.
> 3. Then, in heap_delete, it calls ExtractReplicaIdentity() to find the
> key of the deleted tuple.
> 3a. ExtractR_I_() checks the identity key attributes from
> RelationGetIndexAttrBitmap(..., INDEX_ATTR_BITMAP_IDENTITY_KEY), which
> also only uses rd_replidindex, and doesn't fall back to the primary
> key index's attributes.
> 3b. If ExtractR_I_() doesn't have identity key attributes, it returns NULL
> 3c. heap_delete thus doesn't have any logical identity attributes to
> log, and treats the delete operation as any non-logical deletion when
> logging the data.
> 4. Finally, the DELETE record gets decoded, and the logical plugin
> finds out that no logical key data was included, and promptly ERRORs
> out.
>
> The attached patch is a blind shot that I suspect will fix the issue.
>
>
> Kind regards,
>
> Matthias van de Meent
> Databricks (https://www.databricks.com)
> <v1-0001-Repack-Fix-replica-identity-index-check.patch>
After dropping the index, pg_class.relreplident is still 'i', but the corresponding pg_index entry is deleted, so the table is left in a stale state. If we only check whether the REPLICA IDENTITY index is valid in REPACK, that prevents REPACK from starting, but doesn’t resolve the stale state itself.
We cannot assume the intended replacement replica identity after removing an explicitly selected index. For example, the user might want DEFAULT, FULL, or maybe another index. Should we instead prevent dropping of an index while it is used as REPLICA IDENTITY?
The attached diff makes a change in the direction, like this:
```
evantest=# CREATE TABLE t (a INT PRIMARY KEY, b INT, c TEXT);
CREATE TABLE
evantest=# CREATE UNIQUE INDEX i ON t (a);
CREATE INDEX
evantest=# ALTER TABLE t REPLICA IDENTITY USING INDEX i;
ALTER TABLE
evantest=# DROP INDEX i;
ERROR: cannot drop index "i" because it is used as replica identity
HINT: Use ALTER TABLE ... REPLICA IDENTITY to change the table's replica identity first.
```
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] block_drop_index.diff (3.8K, ../../54DABC65-787E-4DA9-895C-140A3CF862CF@gmail.com/2-block_drop_index.diff)
download | inline diff:
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index fd144d783d9..ec3b374f6fa 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1685,6 +1685,17 @@ RemoveRelations(DropStmt *drop)
continue;
}
+ /*
+ * An explicitly selected replica identity must remain available until
+ * the table's replica identity is changed.
+ */
+ if (drop->removeType == OBJECT_INDEX && get_index_isreplident(relOid))
+ ereport(ERROR,
+ errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
+ errmsg("cannot drop index \"%s\" because it is used as replica identity",
+ rel->relname),
+ errhint("Use ALTER TABLE ... REPLICA IDENTITY to change the table's replica identity first."));
+
/*
* Decide if concurrent mode needs to be used here or not. The
* callback retrieved the rel's persistence for us.
diff --git a/src/test/regress/expected/replica_identity.out b/src/test/regress/expected/replica_identity.out
index 1560cd04125..bcfcf1a1b4b 100644
--- a/src/test/regress/expected/replica_identity.out
+++ b/src/test/regress/expected/replica_identity.out
@@ -133,6 +133,10 @@ SELECT count(*) FROM pg_index WHERE indrelid = 'test_replica_identity'::regclass
1
(1 row)
+-- An explicitly selected replica identity index cannot be dropped.
+DROP INDEX test_replica_identity_keyab_key;
+ERROR: cannot drop index "test_replica_identity_keyab_key" because it is used as replica identity
+HINT: Use ALTER TABLE ... REPLICA IDENTITY to change the table's replica identity first.
----
-- Make sure non index cases work
----
@@ -143,6 +147,7 @@ SELECT relreplident FROM pg_class WHERE oid = 'test_replica_identity'::regclass;
d
(1 row)
+DROP INDEX test_replica_identity_keyab_key;
SELECT count(*) FROM pg_index WHERE indrelid = 'test_replica_identity'::regclass AND indisreplident;
count
-------
@@ -169,7 +174,6 @@ Indexes:
"test_replica_identity_expr" UNIQUE, btree (keya, keyb, (3))
"test_replica_identity_hash" hash (nonkey)
"test_replica_identity_keyab" btree (keya, keyb)
- "test_replica_identity_keyab_key" UNIQUE, btree (keya, keyb)
"test_replica_identity_nonkey" UNIQUE, btree (keya, nonkey)
"test_replica_identity_partial" UNIQUE, btree (keya, keyb) WHERE keyb <> '3'::text
"test_replica_identity_unique_defer" UNIQUE CONSTRAINT, btree (keya, keyb) DEFERRABLE
@@ -200,7 +204,6 @@ Indexes:
"test_replica_identity_expr" UNIQUE, btree (keya, keyb, (3))
"test_replica_identity_hash" hash (nonkey)
"test_replica_identity_keyab" btree (keya, keyb)
- "test_replica_identity_keyab_key" UNIQUE, btree (keya, keyb)
"test_replica_identity_nonkey" UNIQUE, btree (keya, nonkey)
"test_replica_identity_partial" UNIQUE, btree (keya, keyb) WHERE keyb <> '3'::text
"test_replica_identity_unique_defer" UNIQUE CONSTRAINT, btree (keya, keyb) DEFERRABLE
diff --git a/src/test/regress/sql/replica_identity.sql b/src/test/regress/sql/replica_identity.sql
index 4ebb097f282..b8d29ca8fa5 100644
--- a/src/test/regress/sql/replica_identity.sql
+++ b/src/test/regress/sql/replica_identity.sql
@@ -65,11 +65,15 @@ SELECT relreplident FROM pg_class WHERE oid = 'test_replica_identity'::regclass;
\d test_replica_identity
SELECT count(*) FROM pg_index WHERE indrelid = 'test_replica_identity'::regclass AND indisreplident;
+-- An explicitly selected replica identity index cannot be dropped.
+DROP INDEX test_replica_identity_keyab_key;
+
----
-- Make sure non index cases work
----
ALTER TABLE test_replica_identity REPLICA IDENTITY DEFAULT;
SELECT relreplident FROM pg_class WHERE oid = 'test_replica_identity'::regclass;
+DROP INDEX test_replica_identity_keyab_key;
SELECT count(*) FROM pg_index WHERE indrelid = 'test_replica_identity'::regclass AND indisreplident;
ALTER TABLE test_replica_identity REPLICA IDENTITY FULL;
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped
2026-08-27 18:26 REPACK (CONCURRENTLY) fails when replica identity index is dropped Nathan Bossart <nathandbossart@gmail.com>
2026-08-27 19:31 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Matthias van de Meent <boekewurm+postgres@gmail.com>
2026-08-28 06:04 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Chao Li <li.evan.chao@gmail.com>
@ 2026-09-01 10:10 ` Antonin Houska <ah@cybertec.at>
2026-09-01 17:53 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Alvaro Herrera <alvherre@kurilemu.de>
0 siblings, 1 reply; 12+ messages in thread
From: Antonin Houska @ 2026-09-01 10:10 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers; alvherre@kurilemu.de
Chao Li <li.evan.chao@gmail.com> wrote:
> After dropping the index, pg_class.relreplident is still 'i', but the corresponding pg_index entry is deleted, so the table is left in a stale state. If we only check whether the REPLICA IDENTITY index is valid in REPACK, that prevents REPACK from starting, but doesn’t resolve the stale state itself.
>
> We cannot assume the intended replacement replica identity after removing an explicitly selected index. For example, the user might want DEFAULT, FULL, or maybe another index. Should we instead prevent dropping of an index while it is used as REPLICA IDENTITY?
>
> The attached diff makes a change in the direction, like this:
> ```
> evantest=# CREATE TABLE t (a INT PRIMARY KEY, b INT, c TEXT);
> CREATE TABLE
> evantest=# CREATE UNIQUE INDEX i ON t (a);
> CREATE INDEX
> evantest=# ALTER TABLE t REPLICA IDENTITY USING INDEX i;
> ALTER TABLE
> evantest=# DROP INDEX i;
> ERROR: cannot drop index "i" because it is used as replica identity
> HINT: Use ALTER TABLE ... REPLICA IDENTITY to change the table's replica identity first.
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.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped
2026-08-27 18:26 REPACK (CONCURRENTLY) fails when replica identity index is dropped Nathan Bossart <nathandbossart@gmail.com>
2026-08-27 19:31 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Matthias van de Meent <boekewurm+postgres@gmail.com>
2026-08-28 06:04 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Chao Li <li.evan.chao@gmail.com>
2026-09-01 10:10 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Antonin Houska <ah@cybertec.at>
@ 2026-09-01 17:53 ` Alvaro Herrera <alvherre@kurilemu.de>
2026-09-10 10:22 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Alvaro Herrera <alvherre@kurilemu.de>
0 siblings, 1 reply; 12+ messages in thread
From: Alvaro Herrera @ 2026-09-01 17:53 UTC (permalink / raw)
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
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. I don't
think such a behavioral change is backpatchable though, so let's confine
us to pg19. The finding that pg_upgrade doesn't preserve the broken
state is good, because we don't have to handle it in any particular way.
I think the proposed implementation is flawed though, because the index
might be dropped indirectly (maybe an opclass or extension is dropped
CASCADE). I think it should happen somewhere in performDeletion() and
friends, which is to say it should happen during doDeletion(), thus the
check should be in index_drop().
Thanks
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Before you were born your parents weren't as boring as they are now. They
got that way paying your bills, cleaning up your room and listening to you
tell them how idealistic you are." -- Charles J. Sykes' advice to teenagers
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped
2026-08-27 18:26 REPACK (CONCURRENTLY) fails when replica identity index is dropped Nathan Bossart <nathandbossart@gmail.com>
2026-08-27 19:31 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Matthias van de Meent <boekewurm+postgres@gmail.com>
2026-08-28 06:04 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Chao Li <li.evan.chao@gmail.com>
2026-09-01 10:10 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Antonin Houska <ah@cybertec.at>
2026-09-01 17:53 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Alvaro Herrera <alvherre@kurilemu.de>
@ 2026-09-10 10:22 ` Alvaro Herrera <alvherre@kurilemu.de>
2026-09-10 18:09 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Antonin Houska <ah@cybertec.at>
0 siblings, 1 reply; 12+ messages in thread
From: Alvaro Herrera @ 2026-09-10 10:22 UTC (permalink / raw)
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
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
Attachments:
[text/x-diff] 0001-Revert-replica-identity-to-default-if-the-index-is-d.patch (3.7K, ../../aqKC6K_QO0OEv3gy@alvherre.pgsql/2-0001-Revert-replica-identity-to-default-if-the-index-is-d.patch)
download | inline diff:
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
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped
2026-08-27 18:26 REPACK (CONCURRENTLY) fails when replica identity index is dropped Nathan Bossart <nathandbossart@gmail.com>
2026-08-27 19:31 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Matthias van de Meent <boekewurm+postgres@gmail.com>
2026-08-28 06:04 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Chao Li <li.evan.chao@gmail.com>
2026-09-01 10:10 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Antonin Houska <ah@cybertec.at>
2026-09-01 17:53 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Alvaro Herrera <alvherre@kurilemu.de>
2026-09-10 10:22 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Alvaro Herrera <alvherre@kurilemu.de>
@ 2026-09-10 18:09 ` Antonin Houska <ah@cybertec.at>
2026-09-11 01:42 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Chao Li <li.evan.chao@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Antonin Houska @ 2026-09-10 18:09 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@kurilemu.de>; +Cc: Chao Li <li.evan.chao@gmail.com>; Matthias van de Meent <boekewurm+postgres@gmail.com>; Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers
Alvaro Herrera <alvherre@kurilemu.de> wrote:
> 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?
Even though users probably do not drop the identity index too often, I think
it's possible that someone tries to drop an index that seems to be
unnecessary, but forgets that it's in use by logical replication. In such
case, I tend to consider ERROR better response than broken replication.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped
2026-08-27 18:26 REPACK (CONCURRENTLY) fails when replica identity index is dropped Nathan Bossart <nathandbossart@gmail.com>
2026-08-27 19:31 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Matthias van de Meent <boekewurm+postgres@gmail.com>
2026-08-28 06:04 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Chao Li <li.evan.chao@gmail.com>
2026-09-01 10:10 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Antonin Houska <ah@cybertec.at>
2026-09-01 17:53 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Alvaro Herrera <alvherre@kurilemu.de>
2026-09-10 10:22 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Alvaro Herrera <alvherre@kurilemu.de>
2026-09-10 18:09 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Antonin Houska <ah@cybertec.at>
@ 2026-09-11 01:42 ` Chao Li <li.evan.chao@gmail.com>
2026-09-11 08:06 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Antonin Houska <ah@cybertec.at>
0 siblings, 1 reply; 12+ messages in thread
From: Chao Li @ 2026-09-11 01:42 UTC (permalink / raw)
To: Antonin Houska <ah@cybertec.at>; +Cc: Alvaro Herrera <alvherre@kurilemu.de>; Matthias van de Meent <boekewurm+postgres@gmail.com>; Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers
> On Sep 11, 2026, at 02:09, Antonin Houska <ah@cybertec.at> wrote:
>
> Alvaro Herrera <alvherre@kurilemu.de> wrote:
>
>> 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?
>
> Even though users probably do not drop the identity index too often, I think
> it's possible that someone tries to drop an index that seems to be
> unnecessary, but forgets that it's in use by logical replication. In such
> case, I tend to consider ERROR better response than broken replication.
>
> --
> Antonin Houska
> Web: https://www.cybertec-postgresql.com
+1
Actually, there was a similar discussion in [1]. In that case, the question was whether setting a table to UNLOGGED should fail when the table is in a publication’s EXCEPT list, or whether PG should silently remove the table from the EXCEPT list and issue a notice to the user. Most people in that discussion, including Amit, seemed to prefer failing the operation. From a user-experience and design-consistency perspective, I think these two cases are quite similar.
[1] https://postgr.es/m/CAA4eK1KHA-mkvtRPKsE-er8ePOnEu59_hxApaQKtr2=2GNOEQA@mail.gmail.com
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped
2026-08-27 18:26 REPACK (CONCURRENTLY) fails when replica identity index is dropped Nathan Bossart <nathandbossart@gmail.com>
2026-08-27 19:31 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Matthias van de Meent <boekewurm+postgres@gmail.com>
2026-08-28 06:04 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Chao Li <li.evan.chao@gmail.com>
2026-09-01 10:10 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Antonin Houska <ah@cybertec.at>
2026-09-01 17:53 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Alvaro Herrera <alvherre@kurilemu.de>
2026-09-10 10:22 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Alvaro Herrera <alvherre@kurilemu.de>
2026-09-10 18:09 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Antonin Houska <ah@cybertec.at>
2026-09-11 01:42 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Chao Li <li.evan.chao@gmail.com>
@ 2026-09-11 08:06 ` Antonin Houska <ah@cybertec.at>
2026-09-11 09:05 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Alvaro Herrera <alvherre@kurilemu.de>
0 siblings, 1 reply; 12+ messages in thread
From: Antonin Houska @ 2026-09-11 08:06 UTC (permalink / raw)
To: Chao Li <li.evan.chao@gmail.com>; +Cc: Alvaro Herrera <alvherre@kurilemu.de>; Matthias van de Meent <boekewurm+postgres@gmail.com>; Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers
Chao Li <li.evan.chao@gmail.com> wrote:
> On Sep 11, 2026, at 02:09, Antonin Houska <ah@cybertec.at> wrote:
> > Alvaro Herrera <alvherre@kurilemu.de> wrote:
> >
> > > 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?
> >
> > Even though users probably do not drop the identity index too often, I think
> > it's possible that someone tries to drop an index that seems to be
> > unnecessary, but forgets that it's in use by logical replication. In such
> > case, I tend to consider ERROR better response than broken replication.
> +1
>
> Actually, there was a similar discussion in [1]. In that case, the question was whether setting a table to UNLOGGED should fail when the table is in a publication’s EXCEPT list, or whether PG should silently remove the table from the EXCEPT list and issue a notice to the user. Most people in that discussion, including Amit, seemed to prefer failing the operation. From a user-experience and design-consistency perspective, I think these two cases are quite similar.
>
> [1] https://postgr.es/m/CAA4eK1KHA-mkvtRPKsE-er8ePOnEu59_hxApaQKtr2=2GNOEQA@mail.gmail.com
I said "broken replication", but actually the missing replica identity index
triggers error even on the *primary*:
postgres=# delete from a where i=1;
ERROR: cannot delete from table "a" because it does not have a replica identity and publishes deletes
HINT: To enable deleting from the table, set REPLICA IDENTITY using ALTER
TABLE.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped
2026-08-27 18:26 REPACK (CONCURRENTLY) fails when replica identity index is dropped Nathan Bossart <nathandbossart@gmail.com>
2026-08-27 19:31 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Matthias van de Meent <boekewurm+postgres@gmail.com>
2026-08-28 06:04 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Chao Li <li.evan.chao@gmail.com>
2026-09-01 10:10 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Antonin Houska <ah@cybertec.at>
2026-09-01 17:53 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Alvaro Herrera <alvherre@kurilemu.de>
2026-09-10 10:22 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Alvaro Herrera <alvherre@kurilemu.de>
2026-09-10 18:09 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Antonin Houska <ah@cybertec.at>
2026-09-11 01:42 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Chao Li <li.evan.chao@gmail.com>
2026-09-11 08:06 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Antonin Houska <ah@cybertec.at>
@ 2026-09-11 09:05 ` Alvaro Herrera <alvherre@kurilemu.de>
0 siblings, 0 replies; 12+ messages in thread
From: Alvaro Herrera @ 2026-09-11 09:05 UTC (permalink / raw)
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
On 2026-Sep-11, Antonin Houska wrote:
> Chao Li <li.evan.chao@gmail.com> wrote:
>
> > On Sep 11, 2026, at 02:09, Antonin Houska <ah@cybertec.at> wrote:
>
> > > Alvaro Herrera <alvherre@kurilemu.de> wrote:
> > >
> > > > 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?
> > >
> > > Even though users probably do not drop the identity index too often, I think
> > > it's possible that someone tries to drop an index that seems to be
> > > unnecessary, but forgets that it's in use by logical replication. In such
> > > case, I tend to consider ERROR better response than broken replication.
>
> > +1
I can't really disagree with this argument, but sadly, due to the way
object drop works, this is tough to implement. If I simply throw an
error in index_drop(), all manner of things are disallowed: most curious
is probably ALTER TABLE .. SET DATA TYPE on a column of the replica
identity, because that wants to transiently drop the index so that it
can be recreated. But of course the worst is DROP TABLE: because each
individual object deletion is carried out oblivious of every other
object deletion, we don't _know_ that the table containing the replica
identity is _also_ being dropped, so we raise an error when the replica
identity index is dropped and the whole DROP TABLE fails.
Maybe a way to do this would be to hack reportDependentObjects() to see
if a replica identity index is in there, and abort the drop if the table
is not also being dropped. (That doesn't fix the ALTER TABLE TYPE
problem though). This sounds too invasive to consider at this stage of
the cycle. Going forward in pg20 we should try to implement something
like that, but it doesn't seem a good way to close the open item.
Maybe it's better to go back to Matthias original fix proposal instead,
or Ewan Young's variation thereof.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"El número de instalaciones de UNIX se ha elevado a 10,
y se espera que este número aumente" (UPM, 1972)
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped
2026-08-27 18:26 REPACK (CONCURRENTLY) fails when replica identity index is dropped Nathan Bossart <nathandbossart@gmail.com>
2026-08-27 19:31 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Matthias van de Meent <boekewurm+postgres@gmail.com>
@ 2026-08-28 08:29 ` Ewan Young <kdbase.hack@gmail.com>
2026-09-11 11:48 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Alvaro Herrera <alvherre@kurilemu.de>
1 sibling, 1 reply; 12+ messages in thread
From: Ewan Young @ 2026-08-28 08:29 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers@postgresql.org, alvherre@kurilemu.de, Chao Li <li.evan.chao@gmail.com>
Hi Matthias,
Thanks for digging into this. I agree with the direction of v1: the check
should use RelationGetReplicaIndex() so it matches what logical decoding
actually requires, rather than GetRelationIdentityOrPK(), which falls back
to the primary key while decoding does not. That mismatch is exactly what
lets a table with a since-dropped REPLICA IDENTITY USING INDEX slip past the
check and then fail during catch-up with "incomplete delete info".
Two problems with the way v1 restructures the deferrable-primary-key case,
though -- it moves that special case out of the "no identity index" branch
and turns it into an unconditional "must have a non-deferrable PK" test:
1) It now rejects a valid, repackable table. With an explicit replica
identity index and no primary key, ident_idx is valid, so we skip the
first error; but RelationGetPrimaryKeyIndex(rel, false) is InvalidOid, so
the second check fires:
CREATE TABLE rt (a int not null, b text);
CREATE UNIQUE INDEX rt_a ON rt(a);
ALTER TABLE rt REPLICA IDENTITY USING INDEX rt_a;
REPACK (CONCURRENTLY) rt;
-- master: ok
-- v1: ERROR: cannot execute REPACK (CONCURRENTLY) on relation "rt"
-- DETAIL: ... does not support deferrable primary keys.
-- HINT: Use ALTER TABLE ... REPLICA IDENTITY USING INDEX ...
The table has no primary key at all (deferrable or otherwise), and the
hint suggests doing exactly what it already did.
2) The genuine deferrable case loses its message. For a table with a
deferrable PK and no explicit identity, RelationGetReplicaIndex() is
InvalidOid, so it hits the first (generic) error, and the deferrable-
specific message in the second check is now unreachable.
So the deferrable special case ends up firing for the wrong table and not
firing for the right one. I think it just wants to stay a reason for "no
identity index", i.e. inside that branch, and the fix can be the one-line
swap that keeps the original structure:
- ident_idx = GetRelationIdentityOrPK(rel);
+ ident_idx = RelationGetReplicaIndex(rel);
if (!OidIsValid(ident_idx))
{
if (OidIsValid(rel->rd_pkindex) && rel->rd_ispkdeferrable)
ereport(... deferrable primary keys ...);
ereport(... no identity index ...);
}
I built that and checked the three cases: explicit identity index without a
PK still repacks, a dropped-identity-index table is now refused up front
with "no identity index" (instead of failing mid-repack), and the deferrable
case keeps its own message. Happy to post it as a patch if useful.
On Chao's suggestion to block DROP INDEX of a replica identity index: it's a
reasonable hardening idea, but I don't think it removes the need to fix the
check here. The guard lives in RemoveRelations(), so it only covers the
DROP INDEX command -- an index backing a constraint is still dropped through
the dependency path, so e.g.
ALTER TABLE t DROP CONSTRAINT t_a_key; -- t_a_key is the RI index
leaves relreplident = 'i' with the index gone, same stale state. (I
confirmed that on a build with that patch.) And pre-existing tables in that
state, from before such a change or via pg_upgrade, would still reach the
old code path. So REPACK needs to handle the state gracefully regardless;
blocking the drop, if wanted, seems like a separate discussion (and a
behavior change, since dropping such an index is allowed today).
For what it's worth I also agree with the note that decoding can't just fall
back to the PK -- the identity is a contract with subscribers, and an 'i'
table deliberately isn't using the PK, so substituting it would log the
wrong columns.
On Fri, Aug 28, 2026 at 3:31 AM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> On Thu, 27 Aug 2026 at 20:26, Nathan Bossart <nathandbossart@gmail.com> wrote:
> >
> > I don't fully understand the mechanics of this one, but here is a
> > reproducer:
> >
> > CREATE TABLE t (a INT PRIMARY KEY, b INT, c TEXT);
> > INSERT INTO t SELECT g, g, repeat('x', 1000) FROM generate_series(1, 1000000) g;
> > CREATE UNIQUE INDEX i ON t (a);
> > ALTER TABLE t REPLICA IDENTITY USING INDEX i;
> > DROP INDEX i;
> > REPACK (CONCURRENTLY) t;
> >
> > -- in a separate session, while REPACK is still running
> > DELETE FROM t WHERE a = 1;
> >
> > This produces the following ERROR from the REPACK command:
> >
> > ERROR: incomplete delete info
> > CONTEXT: slot "pg_repack_34213", output plugin "pgrepack", in the change callback, associated LSN 0/A6CC1ED0
> > REPACK decoding worker
> >
> > I think this can be addressed by verifying the index exists in
> > check_concurrent_repack_requirements() and erroring out if it doesn't.
>
> I think the issue is caused by the following: REPACK's check has the
> incorrect assumption that REPLICA IDENTITY USING INDEX either reverts
> to DEFAULT or falls back to the behaviour of DEFAULT if the identity
> index gets dropped, and thus uses GetRelationIdentityOrPK(), which
> hides a lack of replica identity index. The issue shows up due to the
> following garden path of data flows:
>
> 1. A table with REPLICA IDENTITY USING INDEX doesn't fall back to
> REPLICA IDENTITY DEFAULT once the identity index is dropped.
> 2. In the catcache, the table won't fall back to rd_replidindex =
> pkeyIndex when replident='i', but instead will set
> rd_replidindex=InvalidOid.
> See the tail end of RelationGetIndexList.
> 3. Then, in heap_delete, it calls ExtractReplicaIdentity() to find the
> key of the deleted tuple.
> 3a. ExtractR_I_() checks the identity key attributes from
> RelationGetIndexAttrBitmap(..., INDEX_ATTR_BITMAP_IDENTITY_KEY), which
> also only uses rd_replidindex, and doesn't fall back to the primary
> key index's attributes.
> 3b. If ExtractR_I_() doesn't have identity key attributes, it returns NULL
> 3c. heap_delete thus doesn't have any logical identity attributes to
> log, and treats the delete operation as any non-logical deletion when
> logging the data.
> 4. Finally, the DELETE record gets decoded, and the logical plugin
> finds out that no logical key data was included, and promptly ERRORs
> out.
>
> The attached patch is a blind shot that I suspect will fix the issue.
>
>
> Kind regards,
>
> Matthias van de Meent
> Databricks (https://www.databricks.com)
--
Regards,
Ewan Young
Attachments:
[application/octet-stream] v2-0001-Repack-Fix-replica-identity-index-check.patch (2.5K, ../../CAON2xHOG1VnJxuuNG7oVUwQjiagRdbz87ravnq-fp9NbMjA7gA@mail.gmail.com/2-v2-0001-Repack-Fix-replica-identity-index-check.patch)
download | inline diff:
From a7bdbea9fdbfe4d24e0e971ebfab20d854a2ca44 Mon Sep 17 00:00:00 2001
From: Ewan Young <kdbase.hack@gmail.com>
Date: Fri, 28 Aug 2026 23:40:01 +0800
Subject: [PATCH v2] Repack: Fix replica identity index check
check_concurrent_repack_requirements() obtained the identity index with
GetRelationIdentityOrPK(), which falls back to the primary key when there
is no replica identity index. Logical decoding, however, uses
RelationGetReplicaIndex() and does not fall back to the primary key. So a
table with REPLICA IDENTITY USING INDEX whose index has since been dropped
(relreplident still 'i', but the index gone) passes the check via the PK
fallback, lets the repack start, and then fails during catch-up with
"incomplete delete info" once a delete has to be decoded.
Use RelationGetReplicaIndex() so the check matches what decoding needs, and
keep the existing error structure: the deferrable-primary-key case stays a
reason for "no identity index" (inside the !OidIsValid branch). This avoids
wrongly rejecting a table that has an explicit replica identity index but no
primary key, and keeps the deferrable-specific message for the case it is
meant for.
---
src/backend/commands/repack.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 477c86b2ba6..cc8c52662be 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -924,13 +924,14 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p)
"REPLICA IDENTITY NOTHING" : "REPLICA IDENTITY FULL"));
/*
- * Obtain the replica identity index -- either one that has been set
- * explicitly, or a non-deferrable primary key. If none of these cases
- * apply, the table cannot be repacked concurrently. It might be possible
- * to have repack work with a FULL replica identity; however that requires
- * more work and is not implemented yet.
- */
- ident_idx = GetRelationIdentityOrPK(rel);
+ * Obtain the replica identity index -- the one set explicitly, or the
+ * default non-deferrable primary key. Use RelationGetReplicaIndex(), not
+ * GetRelationIdentityOrPK(): decoding does not fall back to the primary
+ * key, so neither may we. If there is no such index, the table cannot be
+ * repacked concurrently; a FULL replica identity might be workable but is
+ * not implemented yet.
+ */
+ ident_idx = RelationGetReplicaIndex(rel);
if (!OidIsValid(ident_idx))
{
/* This special case warrants its own error message */
--
2.47.3
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped
2026-08-27 18:26 REPACK (CONCURRENTLY) fails when replica identity index is dropped Nathan Bossart <nathandbossart@gmail.com>
2026-08-27 19:31 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Matthias van de Meent <boekewurm+postgres@gmail.com>
2026-08-28 08:29 ` Re: REPACK (CONCURRENTLY) fails when replica identity index is dropped Ewan Young <kdbase.hack@gmail.com>
@ 2026-09-11 11:48 ` Alvaro Herrera <alvherre@kurilemu.de>
0 siblings, 0 replies; 12+ messages in thread
From: Alvaro Herrera @ 2026-09-11 11:48 UTC (permalink / raw)
To: Ewan Young <kdbase.hack@gmail.com>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers@postgresql.org, Chao Li <li.evan.chao@gmail.com>
On 2026-Aug-28, Ewan Young wrote:
> Thanks for digging into this. I agree with the direction of v1: the check
> should use RelationGetReplicaIndex() so it matches what logical decoding
> actually requires, rather than GetRelationIdentityOrPK(), which falls back
> to the primary key while decoding does not. That mismatch is exactly what
> lets a table with a since-dropped REPLICA IDENTITY USING INDEX slip past the
> check and then fail during catch-up with "incomplete delete info".
Right, thanks for the analysis. I agree with this fix (and I can
confirm that an isolationtester spec for the scenario reproduces the
issue as Nathan reported and no longer does anything weird after the
fix), so I have pushed it. I threw in a test case that verifies that
the sequence is rejected.
Now, IMO the behavior of RelationGetIndexList in this regard is broken:
I think it should set up the PK as replica identity when it's been set
to an index which no longer exists. That allows this to work correctly,
and I can see no downside, but didn't spend too much time on that. I'm
not going to propose changing that in pg19, though. We could also
entertain the idea of switching relreplident back to DEFAULT or just
failing the DROP INDEX outright, but of course only for pg20.
Thanks!
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"El destino baraja y nosotros jugamos" (A. Schopenhauer)
^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2026-09-11 11:48 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-08-27 18:26 REPACK (CONCURRENTLY) fails when replica identity index is dropped Nathan Bossart <nathandbossart@gmail.com>
2026-08-27 19:31 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
2026-08-28 06:04 ` Chao Li <li.evan.chao@gmail.com>
2026-09-01 10:10 ` Antonin Houska <ah@cybertec.at>
2026-09-01 17:53 ` Alvaro Herrera <alvherre@kurilemu.de>
2026-09-10 10:22 ` Alvaro Herrera <alvherre@kurilemu.de>
2026-09-10 18:09 ` Antonin Houska <ah@cybertec.at>
2026-09-11 01:42 ` Chao Li <li.evan.chao@gmail.com>
2026-09-11 08:06 ` Antonin Houska <ah@cybertec.at>
2026-09-11 09:05 ` Alvaro Herrera <alvherre@kurilemu.de>
2026-08-28 08:29 ` Ewan Young <kdbase.hack@gmail.com>
2026-09-11 11:48 ` Alvaro Herrera <alvherre@kurilemu.de>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox