Subject: repack: fix a bug to reject deferrable primary key fallback for concurrent mode

From: Chao Li <[email-masked]>
Date: Fri, 17 Apr 2026 11:35:14 +0800

Hi,

I am continuing to test REPACK, and I found another issue.

In check_concurrent_repack_requirements(), if a table has no replica identity index, the code falls back to using the primary key if one exists. The problem is that a deferrable primary key cannot be used for this purpose. WAL generation does not consider a deferrable primary key to be a replica identity, so concurrent mode may not receive enough old tuple information to replay concurrent changes.

I tested this with the following procedure.

1 - Create a table
```
create table t (id int,  v text, primary key (id) deferrable initially deferred);
insert into t values (1, 'a');
```

2 -  Attach a debugger to session 1's backend process. I used vscode. Add a breakpoint at the first process_concurrent_changes() call. This blocks the REPACK process and gives session 2 time to issue a DELETE.

3 - In session 1, issue a repack, it will stop at the breakpoint
```
repack (concurrently) t;
```

4 - In session 2
```
delete from t where id=1;
```

5 - Detach session 1 from the debugger, so that repack continues and tries to re-apply the delete from session 2.

6 - repack fails with:
```
evantest=# repack (concurrently) t;
ERROR:  incomplete delete info
CONTEXT:  slot "repack_96468", output plugin "pgrepack", in the change callback, associated LSN 0/2A5717F0
REPACK decoding worker
```

The error comes from this code in pgrepack.c:
```
		case REORDER_BUFFER_CHANGE_DELETE:
			{
				HeapTuple	oldtuple;

				oldtuple = change->data.tp.oldtuple;

				if (oldtuple == NULL)
					elog(ERROR, "incomplete delete info");

				repack_store_change(ctx, relation, CHANGE_DELETE, oldtuple);
			}
			break;
```

The root cause is that repack.c assumes rel->rd_pkindex is usable as an identity index, but logical decoding does not treat a deferrable primary key as replica identity. As a result, the decoding worker may not get the old tuple needed to re-apply the delete.

To fix the problem, I think we should just not fall back to deferrable primary key in the first place. See the attached patch.

With this patch, repack will quickly for the test:
```
evantest=# repack (concurrently) t;
ERROR:  cannot process relation "t"
HINT:  Relation "t" has a deferrable primary key.
```

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

---

From: "Zhijie Hou (Fujitsu)" <[email-masked]>
Date: Fri, 17 Apr 2026 03:46:31 +0000

On Friday, April 17, 2026 11:35 AM Chao Li <[email-masked]> wrote:
> In check_concurrent_repack_requirements(), if a table has no replica identity
> index, the code falls back to using the primary key if one exists. The problem is
> that a deferrable primary key cannot be used for this purpose. WAL
> generation does not consider a deferrable primary key to be a replica identity,
> so concurrent mode may not receive enough old tuple information to replay
> concurrent changes.
> 
...
> HINT:  Relation "t" has a deferrable primary key.

Good catch!

I think we can use the existing API to identify the index, for example:

diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 67364cc60e3..cc30236f493 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -62,6 +62,7 @@
 #include "miscadmin.h"
 #include "optimizer/optimizer.h"
 #include "pgstat.h"
+#include "replication/logicalrelation.h"
 #include "storage/bufmgr.h"
 #include "storage/lmgr.h"
 #include "storage/predicate.h"
@@ -924,9 +925,7 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p)
 	 * repack work with a FULL replica identity; however that requires more
 	 * work and is not implemented yet.
 	 */
-	ident_idx = RelationGetReplicaIndex(rel);
-	if (!OidIsValid(ident_idx) && OidIsValid(rel->rd_pkindex))
-		ident_idx = rel->rd_pkindex;
+	ident_idx = GetRelationIdentityOrPK();
 	if (!OidIsValid(ident_idx))
 		ereport(ERROR,
 				errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),

And it would be better to add a test for this.

Best Regards,
Hou zj

---

From: Chao Li <[email-masked]>
Date: Fri, 17 Apr 2026 14:17:15 +0800

> On Apr 17, 2026, at 11:46, Zhijie Hou (Fujitsu) <[email-masked]> wrote:
> 

Thanks for pointing out GetRelationIdentityOrPK(). Looks like it wraps RelationGetReplicaIndex and excludes deferrable primary key.

Switched to use GetRelationIdentityOrPK() in v2.

> And it would be better to add a test for this.

I didn’t add the test because I saw there was only 1 test case of checking catalog table in cluster.sql for repack(concurrently). As you asked, I added tests for all checks of check_concurrent_repack_requirements().

PFA v2.

Best regards,

---

From: Antonin Houska <[email-masked]>
Date: Mon, 20 Apr 2026 16:52:15 +0200

> In check_concurrent_repack_requirements(), if a table has no replica identity index, the code falls back to using the primary key if one exists. The problem is that a deferrable primary key cannot…

Thanks for finding it, this is certainly a problem.

I'm just thinking if it's worth a separate error message.
RelationGetIndexList() just ignores the deferrable PK

	if (replident == REPLICA_IDENTITY_DEFAULT && OidIsValid(pkeyIndex) && !pkdeferrable)
		relation->rd_replidindex = pkeyIndex;

and if there's no other suitable index, the result is that there is no
identity index for the table. So the change attached here should be consistent
with this approach.

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com

diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 67364cc60e3..8cfc3fde5c7 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -925,7 +925,8 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p)
 	 * work and is not implemented yet.
 	 */
 	ident_idx = RelationGetReplicaIndex(rel);
-	if (!OidIsValid(ident_idx) && OidIsValid(rel->rd_pkindex))
+	if (!OidIsValid(ident_idx) && OidIsValid(rel->rd_pkindex) &&
+		!rel->rd_ispkdeferrable)
 		ident_idx = rel->rd_pkindex;
 	if (!OidIsValid(ident_idx))
 		ereport(ERROR,

---

From: Chao Li <[email-masked]>
Date: Tue, 21 Apr 2026 10:38:05 +0800

> On Apr 20, 2026, at 22:52, Antonin Houska <[email-masked]> wrote:
> 

Hi Antonin,

Thanks for your review. I guess you read the v1 patch. In v2, I have switched to use GetRelationIdentityOrPK() that Zhijie suggested, which has covered RelationGetIndexList() and all checks, so that code is simplified, and there is no longer a separate error message.

As CFBot asked for, rebased to v3, nothing changed from v2. Can you please take a look at v3 again?

BTW, could you please also take a look at [1] that is a comment-only change for repack.

[1] https://postgr.es/m/[email-masked]

Best regards,

---

From: Antonin Houska <[email-masked]>
Date: Tue, 21 Apr 2026 08:09:42 +0200

> Thanks for your review. I guess you read the v1 patch. In v2, I have switched to use GetRelationIdentityOrPK() that Zhijie suggested, which has covered RelationGetIndexList() and all checks, so tha…

Yes, this looks like the best approach. Sorry for missing v2.

---

From: Yuchen Li <[email-masked]>
Date: Tue, 21 Apr 2026 15:03:39 +0800

> Yes, this looks like the best approach. Sorry for missing v2.
The patch looks good to me. Using GetRelationIdentityOrPK() makes the 
check match the intended replica identity semantics more closely, and 
the added regression coverage looks useful.

Regards,
Yuchen Li

---

From: Chao Li <[email-masked]>
Date: Sun, 26 Apr 2026 15:35:59 +0800

> On Apr 21, 2026, at 14:09, Antonin Houska <[email-masked]> wrote:
> 

Thanks for your reviewing and confirming.

Rebased to v4 as the CF reported a conflict.

Best regards,

---

From: Alvaro Herrera <[email-masked]>
Date: Mon, 27 Apr 2026 18:24:14 +0200

> Rebased to v4 as the CF reported a conflict.

Thanks, pushed it now.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"La grandeza es una experiencia transitoria.  Nunca es consistente.
Depende en gran parte de la imaginación humana creadora de mitos"
(Irulan)

---

From: Tom Lane <[email-masked]>
Date: Thu, 30 Apr 2026 14:54:02 -0400

> Thanks, pushed it now.

So, no sooner had 4b2aa4b39 removed tests of REPACK CONCURRENTLY
from the core regression tests than 832e220d9 put one back.
BF member thorntail is broken again.

			regards, tom lane