On Mon, Dec 15, 2025 at 5:28 PM Chao Li <[email protected]> wrote:
> > On Mon, Dec 15, 2025 at 4:12 PM Bertrand Drouvot < > [email protected]> wrote: > >> Hi, >> >> On Sat, Dec 13, 2025 at 10:37:40AM +0800, Chao Li wrote: >> > On Dec 12, 2025, at 18:29, Daniel Gustafsson <[email protected]> wrote: >> > >> > On 12 Dec 2025, at 11:27, Heikki Linnakangas <[email protected]> wrote: >> > >> > >> > If we're going to bother changing this at all, let's consider reusing >> the >> > buffer. So instead of initStringInfo()+pfree on every tuple, allocate it >> > once and use resetStringInfo(). >> > >> > >> > +1 >> > >> > As Heikki suggested, I switched to using resetStringInfo() in 0002. One >> > thing I’d like to highlight is that this function only uses the >> StringInfo >> > buffer under certain conditions. If we initialize the StringInfo >> > unconditionally, that feels wasteful; if we initialize it lazily, as in >> > 0002, then we need some extra checks to manage its state. >> >> I think that 0002 works fine. >> >> I'm just wondering if: >> >> + StringInfoData buf = {0}; /* mark as uninitialized */ >> >> does not "break" the semantic of "maxlen == 0" means "read-only". >> >> According to the StringInfoData definition comments: >> >> " >> * As a special case, a StringInfoData can be initialized with a read-only >> * string buffer. In this case "data" does not necessarily point at a >> * palloc'd chunk, and management of the buffer storage is the caller's >> * responsibility. maxlen is set to zero to indicate that this is the >> case. >> * Read-only StringInfoDatas cannot be appended to or reset. >> * Also, it is caller's option whether a read-only string buffer has a >> * terminating '\0' or not. This depends on the intended usage." >> >> The patch uses maxlen == 0 to detect "uninitialized", but the >> documentation >> explicitly says maxlen == 0 indicates "read-only". Maybe adjust the doc a >> bit or >> add a buf_initialized bool in gist_page_items instead? >> >> > Hi Bertrand, > > I think your point is valid. Let's not break the current meaning of > maxlen. I added buf_initialized in v3. As 0001 has been pushed, v2-0002 now > becomes v3-0001. > > Oops, forgot the attachment. Here comes it. Best regards, Chao Li (Evan) --------------------- HighGo Software Co., Ltd. https://www.highgo.com/
v3-0001-pageinspect-clean-up-StringInfo-usage-in-gist_pag.patch
Description: Binary data
