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

> On Apr 17, 2026, at 11:46, Zhijie Hou (Fujitsu) <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Friday, April 17, 2026 11:35 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>> 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.
>>
> ...
>> 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.
>> ```
>
> 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),

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,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/