On Fri, May 8, 2026, 14:10 Chao Li <[email protected]> wrote: > > I don’t think this is a serious leak. In this path, pstate and attnamelist > are allocated in CurTransactionContext, and the transaction is committed > immediately after copy_table() finishes, so that memory is reclaimed at > transaction end. Explicitly freeing them would be mostly for code > readability, not to fix a memory leak. So, I am okay to not free them. >
I agree that no additional memory cleanup is needed here. > While tracing the code, I noticed another issue that is probably more > worth addressing. copy_table() currently does: > ``` > copybuf = makeStringInfo(); > ``` > > But copybuf is only used by copy_read_data(), and there it's really just > acting as a small state holder for data, len, and cursor, rather than as a > normal growable StringInfo. That means we do not need to allocate a > StringInfo object or its backing buffer at all. > > It would be cleaner to use a plain StringInfoData and simply reinitialize > or zero it in copy_table(). See the attached diff for the proposed change. > > David Rowley has made several cleanup changes in this area to prefer > stack-allocated StringInfoData, for example > a63bbc811d41b3567eb37fe2636e660a852dbbf2. This change seems consistent with > that direction. > Thanks for the suggestion. The copybuf change looks worthwhile, but perhaps it’s better discussed in a separate thread. -- Shinya Kato >
