From: Chao Li <li.evan.chao@gmail.com>
Message-Id: <FFFE3D19-58DF-4C23-BE84-934FA968F397@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_922E2FCB-4802-4072-94CA-EA9692EBD35E"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: Avoid unnecessary StringInfo allocation in tablesync COPY buffer
Date: Mon, 11 May 2026 15:09:21 +0800
In-Reply-To: <af9ROet_bQOYyr-g@alvherre.pgsql>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>,
 David Rowley <dgrowleyml@gmail.com>,
 Fujii Masao <masao.fujii@gmail.com>
To: =?utf-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>
References: <af9ROet_bQOYyr-g@alvherre.pgsql>
Archived-At: 
 <https://www.postgresql.org/message-id/FFFE3D19-58DF-4C23-BE84-934FA968F397%40gmail.com>
Precedence: bulk


--Apple-Mail=_922E2FCB-4802-4072-94CA-EA9692EBD35E
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> On May 9, 2026, at 23:35, =C3=81lvaro Herrera <alvherre@kurilemu.de> =
wrote:
>=20
> Hello,
>=20
> On 2026-May-09, Chao Li wrote:
>=20
>> I found this issue while reviewing the patch [1] and was suggested =
use
>> a separate thread for the issue.
>>=20
>> In tablesync.c, copy_table() currently does:
>> ```
>>   copybuf =3D makeStringInfo();
>> ```
>>=20
>> But copybuf is only used by copy_read_data(), and there it's really
>> just acting as a small state holder for data, len, and cursor, rather
>> than as a normal growable StringInfo.
>=20
> I find this coding pattern weird and ugly and confusing.  If what we
> need is three variables, shouldn't we have three variables instead of
> this strange misuse of the StringInfo abstraction?
>=20

Yep, I first considered adding a file-local structure, but decided to =
keep the changes minimal in the first version.

In v2, I switched to using a small file-local CopyBuf structure.

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


--Apple-Mail=_922E2FCB-4802-4072-94CA-EA9692EBD35E
Content-Disposition: attachment;
	filename=v2-0001-Use-simple-struct-for-table-sync-COPY-buffer-stat.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v2-0001-Use-simple-struct-for-table-sync-COPY-buffer-stat.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=20f506b5bc16802434db1999c3d06a4ec95a3c6f18=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20"Chao=20Li=20(Evan)"=20<lic@highgo.com>=0A=
Date:=20Sat,=209=20May=202026=2014:00:02=20+0800=0ASubject:=20[PATCH=20=
v2]=20Use=20simple=20struct=20for=20table=20sync=20COPY=20buffer=20state=0A=
MIME-Version:=201.0=0AContent-Type:=20text/plain;=20charset=3DUTF-8=0A=
Content-Transfer-Encoding:=208bit=0A=0Acopy_read_data()=20only=20needs=20=
to=20track=20the=20COPY=20buffer's=20data=20pointer,=0Alength,=20and=20=
cursor=20position.=20=20Replace=20the=20StringInfo=20with=20a=20small=20=
local=0Astruct=20containing=20just=20those=20fields.=0A=0AAuthor:=20Chao=20=
Li=20<lic@highgo.com>=0AReviewed-by:=20=C3=81lvaro=20Herrera=20=
<alvherre@kurilemu.de>=0ADiscussion:=20=
https://postgr.es/m/5B2C9B4C-EAE6-4F21-AF99-613A561D26DC@gmail.com=0A---=0A=
=20src/backend/replication/logical/tablesync.c=20|=2029=20=
+++++++++++++--------=0A=201=20file=20changed,=2018=20insertions(+),=20=
11=20deletions(-)=0A=0Adiff=20--git=20=
a/src/backend/replication/logical/tablesync.c=20=
b/src/backend/replication/logical/tablesync.c=0Aindex=20=
eb718114297..e2fc37ae2c9=20100644=0A---=20=
a/src/backend/replication/logical/tablesync.c=0A+++=20=
b/src/backend/replication/logical/tablesync.c=0A@@=20-126,7=20+126,14=20=
@@=0A=20=0A=20List=09=20=20=20*table_states_not_ready=20=3D=20NIL;=0A=20=0A=
-static=20StringInfo=20copybuf=20=3D=20NULL;=0A+typedef=20struct=20=
CopyBuf=0A+{=0A+=09char=09=20=20=20*data;=0A+=09int=09=09=09len;=0A+=09=
int=09=09=09cursor;=0A+}=09=09=09CopyBuf;=0A+=0A+static=20CopyBuf=20=
copybuf;=0A=20=0A=20/*=0A=20=20*=20Wait=20until=20the=20relation=20sync=20=
state=20is=20set=20in=20the=20catalog=20to=20the=20expected=0A@@=20=
-649,13=20+656,13=20@@=20copy_read_data(void=20*outbuf,=20int=20minread,=20=
int=20maxread)=0A=20=09int=09=09=09avail;=0A=20=0A=20=09/*=20If=20there=20=
are=20some=20leftover=20data=20from=20previous=20read,=20use=20it.=20*/=0A=
-=09avail=20=3D=20copybuf->len=20-=20copybuf->cursor;=0A+=09avail=20=3D=20=
copybuf.len=20-=20copybuf.cursor;=0A=20=09if=20(avail)=0A=20=09{=0A=20=09=
=09if=20(avail=20>=20maxread)=0A=20=09=09=09avail=20=3D=20maxread;=0A-=09=
=09memcpy(outbuf,=20&copybuf->data[copybuf->cursor],=20avail);=0A-=09=09=
copybuf->cursor=20+=3D=20avail;=0A+=09=09memcpy(outbuf,=20=
&copybuf.data[copybuf.cursor],=20avail);=0A+=09=09copybuf.cursor=20+=3D=20=
avail;=0A=20=09=09maxread=20-=3D=20avail;=0A=20=09=09bytesread=20+=3D=20=
avail;=0A=20=09}=0A@@=20-680,16=20+687,16=20@@=20copy_read_data(void=20=
*outbuf,=20int=20minread,=20int=20maxread)=0A=20=09=09=09else=0A=20=09=09=
=09{=0A=20=09=09=09=09/*=20Process=20the=20data=20*/=0A-=09=09=09=09=
copybuf->data=20=3D=20buf;=0A-=09=09=09=09copybuf->len=20=3D=20len;=0A-=09=
=09=09=09copybuf->cursor=20=3D=200;=0A+=09=09=09=09copybuf.data=20=3D=20=
buf;=0A+=09=09=09=09copybuf.len=20=3D=20len;=0A+=09=09=09=09=
copybuf.cursor=20=3D=200;=0A=20=0A-=09=09=09=09avail=20=3D=20=
copybuf->len=20-=20copybuf->cursor;=0A+=09=09=09=09avail=20=3D=20=
copybuf.len=20-=20copybuf.cursor;=0A=20=09=09=09=09if=20(avail=20>=20=
maxread)=0A=20=09=09=09=09=09avail=20=3D=20maxread;=0A-=09=09=09=09=
memcpy(outbuf,=20&copybuf->data[copybuf->cursor],=20avail);=0A+=09=09=09=09=
memcpy(outbuf,=20&copybuf.data[copybuf.cursor],=20avail);=0A=20=09=09=09=09=
outbuf=20=3D=20(char=20*)=20outbuf=20+=20avail;=0A-=09=09=09=09=
copybuf->cursor=20+=3D=20avail;=0A+=09=09=09=09copybuf.cursor=20+=3D=20=
avail;=0A=20=09=09=09=09maxread=20-=3D=20avail;=0A=20=09=09=09=09=
bytesread=20+=3D=20avail;=0A=20=09=09=09}=0A@@=20-1199,7=20+1206,7=20@@=20=
copy_table(Relation=20rel)=0A=20=09=09=09=09=09=09lrel.nspname,=20=
lrel.relname,=20res->err)));=0A=20=09walrcv_clear_result(res);=0A=20=0A-=09=
copybuf=20=3D=20makeStringInfo();=0A+=09memset(&copybuf,=200,=20=
sizeof(copybuf));=0A=20=0A=20=09pstate=20=3D=20make_parsestate(NULL);=0A=20=
=09(void)=20addRangeTableEntryForRelation(pstate,=20rel,=20=
AccessShareLock,=0A--=20=0A2.50.1=20(Apple=20Git-155)=0A=0A=

--Apple-Mail=_922E2FCB-4802-4072-94CA-EA9692EBD35E--