> On Oct 15, 2026, at 20:49, David Rowley <[email protected]> wrote:
>
> On Fri, 16 Oct 2026 at 01:16, Chao Li <[email protected]> wrote:
>> I tried StringInfo first. A few minor reasons led me to choose the v1
>> approach:
>>
>> * v1 made fewer changes to the existing code.
>> * v1 was slightly faster in my test. With debug and assertions disabled, v1
>> took about 20 ms and StringInfo about 24 ms, averaged over multiple runs.
>
> I doubt that matters very much. I think your test case isn't that
> realistic anyway, as it uses InvalidOid. I tried exercising it with:
> SELECT length(oidvectortypes(repeat('1043 ', 100)::oidvector));
>
> Which gives:
>
> master: tps = 9427.119361
> v1: tps = 9684.447542
> v2: tps = 9681.868182
> v2b: tps = 9707.819189
I intended to avoid the catalog lookup and focus on the difference in string
building. But I agree that the real improvement in normal use would not be as
significant as my test showed, because valid OIDs require catalog lookups and
such long inputs should be rare.
>
> v2b just replaces the appendStringInfoString(&result, ", "); with
> appendBinaryStringInfo(&result, ", ", 2);
>
Good point. appendStringInfoString() calls appendBinaryStringInfo(), so calling
appendBinaryStringInfo() directly when the length is known should be better.
But since Heikki has already pushed v2, perhaps we can leave it as it is.
I searched the source tree and found several other places where we can replace
appendStringInfoString() with appendBinaryStringInfo(), for example in
ExplainFlushWorkersState():
```
for (int i = 0; i < wstate->num_workers; i++)
{
if (wstate->worker_inited[i])
{
/* This must match previous ExplainOpenSetAsideGroup
call */
ExplainOpenGroup("Worker", NULL, true, es);
appendStringInfoString(es->str,
wstate->worker_str[i].data);
ExplainCloseGroup("Worker", NULL, true, es);
pfree(wstate->worker_str[i].data);
}
}
```
Since the length is already available, we can replace the
appendStringInfoString() call with: appendBinaryStringInfo(es->str,
wstate->worker_str[i].data, wstate->worker_str[i].len);
If this is worth doing, I can prepare a separate patch.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
