From: Chao Li <li.evan.chao@gmail.com>
Message-Id: <5FD0FDC6-EE6C-4368-8B1E-7A6F4917254F@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_03703D5A-F817-48C2-8156-272F618F0BEE"
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: Sun, 26 Apr 2026 15:35:59 +0800
In-Reply-To: <5990.1776751782@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> <48844EC6-0909-41E6-B2C4-F3BF8A2B1AA8@gmail.com>
 <5990.1776751782@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/5FD0FDC6-EE6C-4368-8B1E-7A6F4917254F%40gmail.com>
Precedence: bulk


--Apple-Mail=_03703D5A-F817-48C2-8156-272F618F0BEE
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii


> On Apr 21, 2026, at 14:09, Antonin Houska <ah@cybertec.at> wrote:
>=20
> Chao Li <li.evan.chao@gmail.com> wrote:
>=20
>>> On Apr 20, 2026, at 22:52, Antonin Houska <ah@cybertec.at> wrote:
>>>=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
>> 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.
>=20
> Yes, this looks like the best approach. Sorry for missing v2.
>=20

Thanks for your reviewing and confirming.

Rebased to v4 as the CF reported a conflict.

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


--Apple-Mail=_03703D5A-F817-48C2-8156-272F618F0BEE
Content-Disposition: attachment;
	filename=v4-0001-Reject-deferrable-primary-key-fallback-in-REPACK-.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v4-0001-Reject-deferrable-primary-key-fallback-in-REPACK-.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=2036f1fd2c8f31899d08e0b291d49030c200ad7e32=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20"Chao=20Li=20(Evan)"=20<lic@highgo.com>=0A=
Date:=20Sun,=2026=20Apr=202026=2015:31:24=20+0800=0ASubject:=20[PATCH=20=
v4]=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|=2050=20=
+++++++++++++++++++++++++--=0A=20src/test/regress/sql/cluster.sql=20=20=20=
=20=20=20|=2044=20+++++++++++++++++++++--=0A=203=20files=20changed,=2097=20=
insertions(+),=2012=20deletions(-)=0A=0Adiff=20--git=20=
a/src/backend/commands/repack.c=20b/src/backend/commands/repack.c=0A=
index=20bafdca80810..ce5b99c38b1=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=
96089bb0fa2..031664b95f6=20100644=0A---=20=
a/src/test/regress/expected/cluster.out=0A+++=20=
b/src/test/regress/expected/cluster.out=0A@@=20-796,14=20+796,60=20@@=20=
ORDER=20BY=20o.relname;=0A=20=20clstr_3=0A=20(2=20rows)=0A=20=0A---=20=
concurrently=20disallowed=20in=20catalogs=0A+--=0A+--=20Check=20=
concurrent=20mode=20requirements=0A+--=0A+--=20Disallowed=20in=20=
catalogs=0A=20REPACK=20(CONCURRENTLY)=20pg_class;=0A=20ERROR:=20=20=
cannot=20repack=20relation=20"pg_class"=0A=20HINT:=20=20REPACK=20=
CONCURRENTLY=20is=20not=20supported=20for=20catalog=20relations.=0A---=20=
CONCURRENTLY=20doesn't=20like=20partitioned=20tables=0A+--=20Doesn't=20=
like=20partitioned=20tables=0A=20REPACK=20(CONCURRENTLY)=20clstrpart;=0A=20=
ERROR:=20=20REPACK=20(CONCURRENTLY)=20is=20not=20supported=20for=20=
partitioned=20tables=0A=20HINT:=20=20Consider=20running=20the=20command=20=
on=20individual=20partitions.=0A+--=20Doesn't=20support=20catalog=20=
tables=0A+REPACK=20(CONCURRENTLY)=20pg_class;=0A+ERROR:=20=20cannot=20=
repack=20relation=20"pg_class"=0A+HINT:=20=20REPACK=20CONCURRENTLY=20is=20=
not=20supported=20for=20catalog=20relations.=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+ERROR:=20=
=20cannot=20repack=20relation=20"repack_conc_temp"=0A+HINT:=20=20REPACK=20=
CONCURRENTLY=20is=20only=20allowed=20for=20permanent=20relations.=0A=
+DROP=20TABLE=20repack_conc_temp;=0A+CREATE=20UNLOGGED=20TABLE=20=
repack_conc_unlogged=20(i=20int=20PRIMARY=20KEY);=0A+REPACK=20=
(CONCURRENTLY)=20repack_conc_unlogged;=0A+ERROR:=20=20cannot=20repack=20=
relation=20"repack_conc_unlogged"=0A+HINT:=20=20REPACK=20CONCURRENTLY=20=
is=20only=20allowed=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=206b3219bab94..640b3691f44=20100644=0A---=20=
a/src/test/regress/sql/cluster.sql=0A+++=20=
b/src/test/regress/sql/cluster.sql=0A@@=20-383,12=20+383,52=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=20disallowed=20in=20catalogs=0A+--=0A+--=20Check=20=
concurrent=20mode=20requirements=0A+--=0A+=0A+--=20Disallowed=20in=20=
catalogs=0A=20REPACK=20(CONCURRENTLY)=20pg_class;=0A=20=0A---=20=
CONCURRENTLY=20doesn't=20like=20partitioned=20tables=0A+--=20Doesn't=20=
like=20partitioned=20tables=0A=20REPACK=20(CONCURRENTLY)=20clstrpart;=0A=20=
=0A+--=20Doesn't=20support=20catalog=20tables=0A+REPACK=20(CONCURRENTLY)=20=
pg_class;=0A+=0A+--=20Only=20support=20permanent=20tables,=20temp=20and=20=
unlogged=20tables=20are=20not=20supported=0A+CREATE=20TEMP=20TABLE=20=
repack_conc_temp=20(i=20int=20PRIMARY=20KEY);=0A+REPACK=20(CONCURRENTLY)=20=
repack_conc_temp;=0A+DROP=20TABLE=20repack_conc_temp;=0A+CREATE=20=
UNLOGGED=20TABLE=20repack_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=_03703D5A-F817-48C2-8156-272F618F0BEE--