> On Apr 17, 2026, at 11:46, Zhijie Hou (Fujitsu) <[email protected]> > wrote: > > On Friday, April 17, 2026 11:35 AM Chao Li <[email protected]> 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/
v2-0001-Reject-deferrable-primary-key-fallback-in-REPACK-.patch
Description: Binary data
