Re: Avoid repeated scans of the output buffer in oidvectortypes()

> On Oct 15, 2026, at 20:49, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>
> On Fri, 16 Oct 2026 at 01:16, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> 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/