From: Chao Li <li.evan.chao@gmail.com>
Message-Id: <48844EC6-0909-41E6-B2C4-F3BF8A2B1AA8@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_6A779B1E-80A3-4346-9CFF-A4BCEE29F2E8"
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: Tue, 21 Apr 2026 10:38:05 +0800
In-Reply-To: <65564.1776696735@localhost>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>,
 "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>,
 Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Antonin Houska <ah@cybertec.at>
References: <10DD5E13-B45D-44F1-BE08-C63E00ABCAC0@gmail.com>
 <65564.1776696735@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/48844EC6-0909-41E6-B2C4-F3BF8A2B1AA8%40gmail.com>
Precedence: bulk


--Apple-Mail=_6A779B1E-80A3-4346-9CFF-A4BCEE29F2E8
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii


> On Apr 20, 2026, at 22:52, Antonin Houska <ah@cybertec.at> wrote:
>=20
> Chao Li <li.evan.chao@gmail.com> wrote:
>=20
>> 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
> Thanks for finding it, this is certainly a problem.
>=20
> I'm just thinking if it's worth a separate error message.
> RelationGetIndexList() just ignores the deferrable PK
>=20
> if (replident =3D=3D REPLICA_IDENTITY_DEFAULT && OidIsValid(pkeyIndex) =
&& !pkdeferrable)
> relation->rd_replidindex =3D pkeyIndex;
>=20
> 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.
>=20
> --=20
> Antonin Houska
> Web: https://www.cybertec-postgresql.com
>=20

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/06F150E0-FDE9-4FC6-9EEF-79A759430471@gmail.com

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


--Apple-Mail=_6A779B1E-80A3-4346-9CFF-A4BCEE29F2E8
Content-Disposition: attachment;
	filename=v3-0001-Reject-deferrable-primary-key-fallback-in-REPACK-.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v3-0001-Reject-deferrable-primary-key-fallback-in-REPACK-.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=20fdca43ea733a269a5fdb7621b26d16841eddb0c2=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=
v3]=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>=0AReviewed-by:=20=
Antonin=20Houska=20<ah@cybertec.at>=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=
71270134985..e7cb89c5298=20100644=0A---=20=
a/src/test/regress/expected/cluster.out=0A+++=20=
b/src/test/regress/expected/cluster.out=0A@@=20-822,10=20+822,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=206746236ffec..e5e369050bb=20100644=0A---=20=
a/src/test/regress/sql/cluster.sql=0A+++=20=
b/src/test/regress/sql/cluster.sql=0A@@=20-393,9=20+393,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=_6A779B1E-80A3-4346-9CFF-A4BCEE29F2E8--