> On May 9, 2026, at 02:05, Shinya Kato <[email protected]> wrote:
>
>
>
> 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.
>
Sound fair. Let me post it to a separate thread.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
