On Mon, 9 Oct 2023 at 17:37, Tom Lane <t...@sss.pgh.pa.us> wrote: > Sorry for not having paid more attention to this thread ... but > I'm pretty desperately unhappy with the patch as-pushed. I agree > with the criticism that this is a very repetitive coding pattern > that could have used a macro. But my real problem with this: > > + buf.data = VARDATA_ANY(sstate); > + buf.len = VARSIZE_ANY_EXHDR(sstate); > + buf.maxlen = 0; > + buf.cursor = 0; > > is that it totally breaks the StringInfo API without even > attempting to fix the API specs that it falsifies, > particularly this in stringinfo.h: > > * maxlen is the allocated size in bytes of 'data', i.e. the maximum > * string size (including the terminating '\0' char) that we > can > * currently store in 'data' without having to reallocate > * more space. We must always have maxlen > len. > > I could see inventing a notion of a "read-only StringInfo" > to legitimize what you've done here, but you didn't bother > to try. I do not like this one bit. This is a fairly > fundamental API and we shouldn't be so cavalier about > breaking it.
You originally called the centralised logic a "loaded foot-gun" [1], but now you're complaining about a lack of loaded foot-gun and want a macro? Which part did I misunderstand? Enlighten me, please. Here are some more thoughts on how we could improve this: 1. Adjust the definition of StringInfoData.maxlen to define that -1 means the StringInfoData's buffer is externally managed. 2. Adjust enlargeStringInfo() to add a check for maxlen = -1 and have it palloc, say, pg_next_pow2(str->len * 2) bytes and memcpy the existing (externally managed string) into the newly palloc'd buffer. 3. Add a new function along the lines of what I originally proposed to allow init of a StringInfoData using an existing allocated string which sets maxlen = -1. 4. Update all the existing places, including the ones I just committed (plus the ones you committed in ba1e066e4) to make use of the function added in #3. Better ideas? David [1] https://postgr.es/m/770055.1676183...@sss.pgh.pa.us
