From: Chao Li <li.evan.chao@gmail.com>
Message-Id: <EF04ED8D-89AF-44DF-8BFD-C48B38E1D9C1@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_F70222E3-9749-4E40-ADFB-CFFF9657F2AD"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: repack: fix a bug to reject deferrable primary key fallback for
 concurrent mode
Date: Fri, 17 Apr 2026 14:17:15 +0800
In-Reply-To: 
 <TYRPR01MB14195C23394DC17ABBD321B9694202@TYRPR01MB14195.jpnprd01.prod.outlook.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
References: <10DD5E13-B45D-44F1-BE08-C63E00ABCAC0@gmail.com>
 <TYRPR01MB14195C23394DC17ABBD321B9694202@TYRPR01MB14195.jpnprd01.prod.outlook.com>
Archived-At: 
 <https://www.postgresql.org/message-id/EF04ED8D-89AF-44DF-8BFD-C48B38E1D9C1%40gmail.com>
Precedence: bulk


--Apple-Mail=_F70222E3-9749-4E40-ADFB-CFFF9657F2AD
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> On Apr 17, 2026, at 11:46, Zhijie Hou (Fujitsu) =
<houzj.fnst@fujitsu.com> wrote:
>=20
> On Friday, April 17, 2026 11:35 AM Chao Li <li.evan.chao@gmail.com> =
wrote:
>> I am continuing to test REPACK, and I found another issue.
>>=20
>> 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.
>>=20
>> I tested this with the following procedure.
>>=20
> ...
>> With this patch, repack will quickly for the test:
>> ```
>> evantest=3D# repack (concurrently) t;
>> ERROR:  cannot process relation "t"
>> HINT:  Relation "t" has a deferrable primary key.
>> ```
>=20
> Good catch!
>=20
> I think we can use the existing API to identify the index, for =
example:
>=20
> 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 =3D RelationGetReplicaIndex(rel);
> - if (!OidIsValid(ident_idx) && OidIsValid(rel->rd_pkindex))
> - ident_idx =3D rel->rd_pkindex;
> + ident_idx =3D 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.

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

I didn=E2=80=99t 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/


--Apple-Mail=_F70222E3-9749-4E40-ADFB-CFFF9657F2AD
Content-Disposition: attachment;
	filename=v2-0001-Reject-deferrable-primary-key-fallback-in-REPACK-.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v2-0001-Reject-deferrable-primary-key-fallback-in-REPACK-.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=206ced3f3010d778dd18dd962b48dbebd0ecebbda3=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20"Chao=20Li=20(Evan)"=20<lic@highgo.com>=0A=
Date:=20Fri,=2017=20Apr=202026=2011:12:36=20+0800=0ASubject:=20[PATCH=20=
v2]=20Reject=20deferrable=20primary=20key=20fallback=20in=20REPACK=0A=20=
CONCURRENTLY=0A=0AREPACK=20CONCURRENTLY=20uses=20logical=20decoding=20to=20=
collect=20concurrent=0Achanges=20and=20then=20replays=20them=20on=20the=20=
new=20heap.=20To=20locate=20rows=20for=0AUPDATE=20and=20DELETE=20replay,=20=
it=20requires=20an=20identity=20index.=0A=0AWhen=20=
RelationGetReplicaIndex()=20returned=20InvalidOid,=20the=20code=20fell=0A=
back=20to=20rel->rd_pkindex=20if=20a=20primary=20key=20existed.=20That=20=
is=20not=20safe=20for=0Adeferrable=20primary=20keys.=20Such=20indexes=20=
are=20not=20considered=20replica=0Aidentity=20indexes=20by=20WAL=20=
generation,=20so=20logical=20decoding=20may=20not=20provide=0Athe=20old=20=
tuple=20needed=20by=20the=20repack=20output=20plugin.=0A=0AThis=20can=20=
make=20replay=20fail=20later=20with=20errors=20such=20as=20"incomplete=20=
delete=0Ainfo"=20from=20the=20decoding=20worker.=0A=0AThis=20change=20=
switches=20to=20use=20GetRelationIdentityOrPK()=20that=20excludes=0A=
deferrable=20primary=20key.=0A=0AAuthor:=20Chao=20Li=20<lic@highgo.com>=0A=
Reviewed-by:=20Zhijie=20Hou=20<houzj.fnst@fujitsu.com>=0ADiscussion:=20=
https://postgr.es/m/10DD5E13-B45D-44F1-BE08-C63E00ABCAC0@gmail.com=0A---=0A=
=20src/backend/commands/repack.c=20=20=20=20=20=20=20=20=20|=2015=20=
+++++----=0A=20src/test/regress/expected/cluster.out=20|=2044=20=
++++++++++++++++++++++++++-=0A=20src/test/regress/sql/cluster.sql=20=20=20=
=20=20=20|=2039=20+++++++++++++++++++++++-=0A=203=20files=20changed,=20=
88=20insertions(+),=2010=20deletions(-)=0A=0Adiff=20--git=20=
a/src/backend/commands/repack.c=20b/src/backend/commands/repack.c=0A=
index=2067364cc60e3..76d84180acc=20100644=0A---=20=
a/src/backend/commands/repack.c=0A+++=20b/src/backend/commands/repack.c=0A=
@@=20-62,6=20+62,7=20@@=0A=20#include=20"miscadmin.h"=0A=20#include=20=
"optimizer/optimizer.h"=0A=20#include=20"pgstat.h"=0A+#include=20=
"replication/logicalrelation.h"=0A=20#include=20"storage/bufmgr.h"=0A=20=
#include=20"storage/lmgr.h"=0A=20#include=20"storage/predicate.h"=0A@@=20=
-919,14=20+920,12=20@@=20check_concurrent_repack_requirements(Relation=20=
rel,=20Oid=20*ident_idx_p)=0A=20=0A=20=09/*=0A=20=09=20*=20Obtain=20the=20=
replica=20identity=20index=20--=20either=20one=20that=20has=20been=20set=0A=
-=09=20*=20explicitly,=20or=20the=20primary=20key.=20=20If=20none=20of=20=
these=20cases=20apply,=20the=0A-=09=20*=20table=20cannot=20be=20repacked=20=
concurrently.=20=20It=20might=20be=20possible=20to=20have=0A-=09=20*=20=
repack=20work=20with=20a=20FULL=20replica=20identity;=20however=20that=20=
requires=20more=0A-=09=20*=20work=20and=20is=20not=20implemented=20yet.=0A=
-=09=20*/=0A-=09ident_idx=20=3D=20RelationGetReplicaIndex(rel);=0A-=09if=20=
(!OidIsValid(ident_idx)=20&&=20OidIsValid(rel->rd_pkindex))=0A-=09=09=
ident_idx=20=3D=20rel->rd_pkindex;=0A+=09=20*=20explicitly,=20or=20a=20=
non-deferrable=20primary=20key.=20=20If=20none=20of=20these=20cases=0A+=09=
=20*=20apply,=20the=20table=20cannot=20be=20repacked=20concurrently.=20=20=
It=20might=20be=20possible=0A+=09=20*=20to=20have=20repack=20work=20with=20=
a=20FULL=20replica=20identity;=20however=20that=20requires=0A+=09=20*=20=
more=20work=20and=20is=20not=20implemented=20yet.=0A+=09=20*/=0A+=09=
ident_idx=20=3D=20GetRelationIdentityOrPK(rel);=0A=20=09if=20=
(!OidIsValid(ident_idx))=0A=20=09=09ereport(ERROR,=0A=20=09=09=09=09=
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),=0Adiff=20--git=20=
a/src/test/regress/expected/cluster.out=20=
b/src/test/regress/expected/cluster.out=0Aindex=20=
6127b215a86..0ba87972712=20100644=0A---=20=
a/src/test/regress/expected/cluster.out=0A+++=20=
b/src/test/regress/expected/cluster.out=0A@@=20-806,10=20+806,52=20@@=20=
ORDER=20BY=20o.relname;=0A=20=20clstr_3=0A=20(2=20rows)=0A=20=0A---=20=
concurrently=0A+--=0A+--=20Check=20concurrent=20mode=20requirements=0A=
+--=0A+--=20Doesn't=20support=20catalog=20tables=0A=20REPACK=20=
(CONCURRENTLY)=20pg_class;=0A=20ERROR:=20=20cannot=20repack=20relation=20=
"pg_class"=0A=20HINT:=20=20REPACK=20CONCURRENTLY=20is=20not=20supported=20=
for=20catalog=20relations.=0A+--=20Only=20support=20permanent=20tables,=20=
temp=20and=20unlogged=20tables=20are=20not=20supported=0A+CREATE=20TEMP=20=
TABLE=20repack_conc_temp=20(i=20int=20PRIMARY=20KEY);=0A+REPACK=20=
(CONCURRENTLY)=20repack_conc_temp;=0A+ERROR:=20=20cannot=20repack=20=
relation=20"repack_conc_temp"=0A+HINT:=20=20REPACK=20CONCURRENTLY=20is=20=
only=20allowed=20for=20permanent=20relations.=0A+DROP=20TABLE=20=
repack_conc_temp;=0A+CREATE=20UNLOGGED=20TABLE=20repack_conc_unlogged=20=
(i=20int=20PRIMARY=20KEY);=0A+REPACK=20(CONCURRENTLY)=20=
repack_conc_unlogged;=0A+ERROR:=20=20cannot=20repack=20relation=20=
"repack_conc_unlogged"=0A+HINT:=20=20REPACK=20CONCURRENTLY=20is=20only=20=
allowed=20for=20permanent=20relations.=0A+DROP=20TABLE=20=
repack_conc_unlogged;=0A+--=20Doesn't=20support=20tables=20without=20a=20=
primary=20key=20or=20replica=20identity=20index=0A+CREATE=20TABLE=20=
repack_conc_noident=20(i=20int);=0A+REPACK=20(CONCURRENTLY)=20=
repack_conc_noident;=0A+ERROR:=20=20cannot=20process=20relation=20=
"repack_conc_noident"=0A+HINT:=20=20Relation=20"repack_conc_noident"=20=
has=20no=20identity=20index.=0A+DROP=20TABLE=20repack_conc_noident;=0A=
+--=20Doesn't=20support=20TOAST=20tables=20directly=0A+CREATE=20TABLE=20=
repack_conc_toast=20(t=20text);=0A+SELECT=20reltoastrelid::regclass=20AS=20=
toast_rel=0A+FROM=20pg_class=20WHERE=20oid=20=3D=20=
'repack_conc_toast'::regclass=20\gset=0A+\set=20VERBOSITY=20sqlstate=0A=
+REPACK=20(CONCURRENTLY)=20:toast_rel;=0A+ERROR:=20=200A000=0A+\set=20=
VERBOSITY=20default=0A+DROP=20TABLE=20repack_conc_toast;=0A+--=20Doesn't=20=
support=20tables=20with=20REPLICA=20IDENTITY=20NOTHING,=20even=20if=20=
they=20have=20a=20primary=20key=0A+CREATE=20TABLE=20repack_conc_nothing=20=
(i=20int=20PRIMARY=20KEY);=0A+ALTER=20TABLE=20repack_conc_nothing=20=
REPLICA=20IDENTITY=20NOTHING;=0A+REPACK=20(CONCURRENTLY)=20=
repack_conc_nothing;=0A+ERROR:=20=20cannot=20repack=20relation=20=
"repack_conc_nothing"=0A+HINT:=20=20Relation=20"repack_conc_nothing"=20=
has=20insufficient=20replication=20identity.=0A+DROP=20TABLE=20=
repack_conc_nothing;=0A+--=20Doesn't=20support=20tables=20with=20=
deferrable=20primary=20keys=0A+CREATE=20TABLE=20repack_conc_deferrable=20=
(i=20int=20PRIMARY=20KEY=20DEFERRABLE);=0A+REPACK=20(CONCURRENTLY)=20=
repack_conc_deferrable;=0A+ERROR:=20=20cannot=20process=20relation=20=
"repack_conc_deferrable"=0A+HINT:=20=20Relation=20=
"repack_conc_deferrable"=20has=20no=20identity=20index.=0A+DROP=20TABLE=20=
repack_conc_deferrable;=0A=20--=20clean=20up=0A=20DROP=20TABLE=20=
clustertest;=0A=20DROP=20TABLE=20clstr_1;=0Adiff=20--git=20=
a/src/test/regress/sql/cluster.sql=20b/src/test/regress/sql/cluster.sql=0A=
index=20d14063a9683..99ff3739c60=20100644=0A---=20=
a/src/test/regress/sql/cluster.sql=0A+++=20=
b/src/test/regress/sql/cluster.sql=0A@@=20-386,9=20+386,46=20@@=20JOIN=20=
relnodes_new=20n=20ON=20o.relname=20=3D=20n.relname=0A=20WHERE=20=
o.relfilenode=20<>=20n.relfilenode=0A=20ORDER=20BY=20o.relname;=0A=20=0A=
---=20concurrently=0A+--=0A+--=20Check=20concurrent=20mode=20=
requirements=0A+--=0A+=0A+--=20Doesn't=20support=20catalog=20tables=0A=20=
REPACK=20(CONCURRENTLY)=20pg_class;=0A=20=0A+--=20Only=20support=20=
permanent=20tables,=20temp=20and=20unlogged=20tables=20are=20not=20=
supported=0A+CREATE=20TEMP=20TABLE=20repack_conc_temp=20(i=20int=20=
PRIMARY=20KEY);=0A+REPACK=20(CONCURRENTLY)=20repack_conc_temp;=0A+DROP=20=
TABLE=20repack_conc_temp;=0A+CREATE=20UNLOGGED=20TABLE=20=
repack_conc_unlogged=20(i=20int=20PRIMARY=20KEY);=0A+REPACK=20=
(CONCURRENTLY)=20repack_conc_unlogged;=0A+DROP=20TABLE=20=
repack_conc_unlogged;=0A+=0A+--=20Doesn't=20support=20tables=20without=20=
a=20primary=20key=20or=20replica=20identity=20index=0A+CREATE=20TABLE=20=
repack_conc_noident=20(i=20int);=0A+REPACK=20(CONCURRENTLY)=20=
repack_conc_noident;=0A+DROP=20TABLE=20repack_conc_noident;=0A+=0A+--=20=
Doesn't=20support=20TOAST=20tables=20directly=0A+CREATE=20TABLE=20=
repack_conc_toast=20(t=20text);=0A+SELECT=20reltoastrelid::regclass=20AS=20=
toast_rel=0A+FROM=20pg_class=20WHERE=20oid=20=3D=20=
'repack_conc_toast'::regclass=20\gset=0A+\set=20VERBOSITY=20sqlstate=0A=
+REPACK=20(CONCURRENTLY)=20:toast_rel;=0A+\set=20VERBOSITY=20default=0A=
+DROP=20TABLE=20repack_conc_toast;=0A+=0A+--=20Doesn't=20support=20=
tables=20with=20REPLICA=20IDENTITY=20NOTHING,=20even=20if=20they=20have=20=
a=20primary=20key=0A+CREATE=20TABLE=20repack_conc_nothing=20(i=20int=20=
PRIMARY=20KEY);=0A+ALTER=20TABLE=20repack_conc_nothing=20REPLICA=20=
IDENTITY=20NOTHING;=0A+REPACK=20(CONCURRENTLY)=20repack_conc_nothing;=0A=
+DROP=20TABLE=20repack_conc_nothing;=0A+=0A+--=20Doesn't=20support=20=
tables=20with=20deferrable=20primary=20keys=0A+CREATE=20TABLE=20=
repack_conc_deferrable=20(i=20int=20PRIMARY=20KEY=20DEFERRABLE);=0A=
+REPACK=20(CONCURRENTLY)=20repack_conc_deferrable;=0A+DROP=20TABLE=20=
repack_conc_deferrable;=0A+=0A=20--=20clean=20up=0A=20DROP=20TABLE=20=
clustertest;=0A=20DROP=20TABLE=20clstr_1;=0A--=20=0A2.50.1=20(Apple=20=
Git-155)=0A=0A=

--Apple-Mail=_F70222E3-9749-4E40-ADFB-CFFF9657F2AD--