| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, David Rowley <dgrowleyml(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Subject: | Re: Avoid unnecessary StringInfo allocation in tablesync COPY buffer |
| Date: | 2026-05-11 07:09:21 |
| Message-ID: | FFFE3D19-58DF-4C23-BE84-934FA968F397@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On May 9, 2026, at 23:35, Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
>
> Hello,
>
> On 2026-May-09, Chao Li wrote:
>
>> I found this issue while reviewing the patch [1] and was suggested use
>> a separate thread for the issue.
>>
>> In tablesync.c, 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.
>
> I find this coding pattern weird and ugly and confusing. If what we
> need is three variables, shouldn't we have three variables instead of
> this strange misuse of the StringInfo abstraction?
>
Yep, I first considered adding a file-local structure, but decided to keep the changes minimal in the first version.
In v2, I switched to using a small file-local CopyBuf structure.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Use-simple-struct-for-table-sync-COPY-buffer-stat.patch | application/octet-stream | 2.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | SATYANARAYANA NARLAPURAM | 2026-05-11 07:12:29 | SUBSCRIPTION SERVER ALTER/DROP operations stuck when user mapping is dropped |
| Previous Message | Ayush Tiwari | 2026-05-11 06:57:43 | Re: Plug-in coverage hole for pglz_decompress() |