| From: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | RE: repack: fix a bug to reject deferrable primary key fallback for concurrent mode |
| Date: | 2026-04-17 03:46:31 |
| Message-ID: | TYRPR01MB14195C23394DC17ABBD321B9694202@TYRPR01MB14195.jpnprd01.prod.outlook.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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),
And it would be better to add a test for this.
Best Regards,
Hou zj
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Xiaopeng Wang | 2026-04-17 03:47:34 | Re: [doc] pg_ctl: fix wrong description for -l |
| Previous Message | Chao Li | 2026-04-17 03:41:37 | Re: repack: fix uninitialized DecodingWorkerShared.initialized |