From: Chao Li <li.evan.chao@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_13E750B7-273F-41A6-B7D9-0A0C7079F7EA"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Avoid unnecessary StringInfo allocation in tablesync COPY buffer
Message-Id: <5B2C9B4C-EAE6-4F21-AF99-613A561D26DC@gmail.com>
Date: Sat, 9 May 2026 14:10:50 +0800
Cc: David Rowley <dgrowleyml@gmail.com>,
 Fujii Masao <masao.fujii@gmail.com>
To: PostgreSQL-development <pgsql-hackers@postgresql.org>
Archived-At: 
 <https://www.postgresql.org/message-id/5B2C9B4C-EAE6-4F21-AF99-613A561D26DC%40gmail.com>
Precedence: bulk


--Apple-Mail=_13E750B7-273F-41A6-B7D9-0A0C7079F7EA
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

Hi,

I found this issue while reviewing the patch [1] and was suggested use a =
separate thread for the issue.

In tablesync.c, copy_table() currently does:
```
   copybuf =3D makeStringInfo();
```

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. That means we do not need to allocate a =
StringInfo object or its backing buffer at all.

It would be cleaner to use a plain StringInfoData and simply =
reinitialize or zero it in copy_table(). See the attached patch for the =
proposed change.

David Rowley has made several cleanup changes in this area to prefer =
stack-allocated StringInfoData, for example =
a63bbc811d41b3567eb37fe2636e660a852dbbf2. This change seems consistent =
with that direction.

[1] =
https://postgr.es/m/CAOzEurQKuy3RiPkd=3D25PEwEzaqHuGvEOf=3DX7vaVzhgNjaukYz=
A@mail.gmail.com

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


--Apple-Mail=_13E750B7-273F-41A6-B7D9-0A0C7079F7EA
Content-Disposition: attachment;
	filename=v1-0001-Avoid-unnecessary-StringInfo-allocation-in-tables.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v1-0001-Avoid-unnecessary-StringInfo-allocation-in-tables.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=20cffb27e65afaf3b7b0202f4e14518a424cf52ae3=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=
v1]=20Avoid=20unnecessary=20StringInfo=20allocation=20in=20tablesync=20=
COPY=0A=20buffer=0A=0Acopybuf=20is=20only=20used=20to=20track=20the=20=
data=20pointer,=20length,=20and=20cursor=20for=0Aleftover=20COPY=20data.=20=
The=20buffer=20allocated=20by=20makeStringInfo()=20is=20not=20used,=0A=
because=20copybuf->data=20is=20later=20replaced=20by=20the=20buffer=20=
returned=20from=0Awalrcv_receive().=0A=0AUse=20a=20static=20=
StringInfoData=20instead=20and=20reset=20it=20before=20starting=20COPY.=0A=
=0AAuthor:=20Chao=20Li=20<lic@highgo.com>=0A---=0A=20=
src/backend/replication/logical/tablesync.c=20|=2022=20=
++++++++++-----------=0A=201=20file=20changed,=2011=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..1dee2480e2f=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,7=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+static=20StringInfoData=20=
copybuf=20=3D=20{0};=0A=20=0A=20/*=0A=20=20*=20Wait=20until=20the=20=
relation=20sync=20state=20is=20set=20in=20the=20catalog=20to=20the=20=
expected=0A@@=20-649,13=20+649,13=20@@=20copy_read_data(void=20*outbuf,=20=
int=20minread,=20int=20maxread)=0A=20=09int=09=09=09avail;=0A=20=0A=20=09=
/*=20If=20there=20are=20some=20leftover=20data=20from=20previous=20read,=20=
use=20it.=20*/=0A-=09avail=20=3D=20copybuf->len=20-=20copybuf->cursor;=0A=
+=09avail=20=3D=20copybuf.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=20=
maxread;=0A-=09=09memcpy(outbuf,=20&copybuf->data[copybuf->cursor],=20=
avail);=0A-=09=09copybuf->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+680,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+1199,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=_13E750B7-273F-41A6-B7D9-0A0C7079F7EA--