On Thursday, October 12, 2023 12:04 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Hi,
>
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > I was thinking about this when skimming the other StringInfo thread a
> > couple of days ago. I wondered if it wouldn't be more convenient to
> > change the convention that all StringInfos are null-terminated: what
> > is really the reason to have them all be like that?
>
> It makes sense for StringInfos containing text, not because the stringinfo.c
> routines need it but because callers inspecting the string will very likely do
> something that expects nul-termination.
> When the StringInfo contains binary data, that argument has little force of
> course.
>
> I could see extending the convention for caller-supplied buffers (as is under
> discussion in the other thread) to say that the caller needn't provide a
> nul-terminated buffer if it is confident that no reader of the StringInfo will need
> that. I'd be even more inclined than before to tie this to a specification that
> such a StringInfo is read-only, though.
>
> In any case, this does not immediately let us jump to the conclusion that it'd be
> safe to use such a convention in apply workers. Aren't the things being passed
> around here usually text strings?
I think the data passed to parallel apply worker is of mixed types. If we see
the data reading logic for it like logicalrep_read_attrs(), it uses both
pq_getmsgint/pq_getmsgbyte/pq_getmsgint(binary) and pq_getmsgstring(text).
> Do you really want to promise that no reader is depending on nul-termination?
I think we could not make an absolute guarantee in this regard, but currently
all the consumer uses pq_getxxx style functions to read the passed data and it
also takes care to read the text stuff(get the length separately e.g.
logicalrep_read_tuple). So it seems OK to release the rule for it.
OTOH, I am not opposed to keeping the rule intact for the apply worker, just to
share the information and to gather opinions from others.
Best Regards,
Hou zj