From: | Sutou Kouhei <kou(at)clear-code(dot)com> |
---|---|
To: | sawada(dot)mshk(at)gmail(dot)com |
Cc: | michael(at)paquier(dot)xyz, david(dot)g(dot)johnston(at)gmail(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us, zhjwpku(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Make COPY format extendable: Extract COPY TO format implementations |
Date: | 2025-10-03 07:06:50 |
Message-ID: | 20251003.160650.355943655010493993.kou@clear-code.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
In <CAD21AoADXWgdizS0mV5w8wdfftDRsm8sUtNW=CzYYS1OhjFD2A(at)mail(dot)gmail(dot)com>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Mon, 15 Sep 2025 10:00:18 -0700,
Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>> > I think we can use a local variable of CopyFormatOptions and memcpy it
>> > to the opts of the returned cstate.
>>
>> It'll work too. Can we proceed this proposal with this
>> approach? Should we collect more opinions before we proceed?
>> If so, Could you add key people in this area to Cc to hear
>> their opinions?
>
> Since we don't have a single decision-maker, we should proceed through
> consensus-building and careful evaluation of each approach. I see that
> several senior hackers are already included in this thread, which is
> excellent. Since you and I, who have been involved in these
> discussions, agreed with this approach, I believe we can proceed with
> this direction. If anyone proposes alternative solutions that we find
> more compelling, we might have to change the approach.
OK. There is no objection for now.
How about the attached patch? The patch uses the approach
only for CopyToStateData. If this looks good, I can do it
for CopyFromStateData too.
This patch splits CopyToStateData to
* CopyToStateData
* CopyToStateInternalData
* CopyToStateBuiltinData
structs.
This is based on the category described in
https://www.postgresql.org/message-id/flat/CAD21AoBa0Wm3C2H12jaqkvLidP2zEhsC%2Bgf%3D3w7XiA4LQnvx0g%40mail.gmail.com#85cb988b0bec243d1e8dce699e02e009
:
> 1. fields used only the core (not by format callback)
> 2. fields used by both the core and format callbacks
> 3. built-in format specific fields (mostly for text and csv)
CopyToStateInternalData is for 1.
CopyToStateData is for 2.
CopyToStateBuiltinData is for 3.
This patch adds CopyToRoutine::CopyToGetStateSize() that
returns size of state struct for the routine. For example,
Built-in formats use sizeof(CopyToStateBuiltinData) for it.
BeginCopyTo() allocates sizeof(CopyToStateInternalData) +
CopyToGetStateSize() size continuous memory and uses the
front part as CopyToStateInternalData and the back part as
CopyToStateData/CopyToStateBuilinData.
Thanks,
--
kou
Attachment | Content-Type | Size |
---|---|---|
0001-Split-CopyToStateData-to-CopyToState-Internal-Builti.patch | text/x-patch | 24.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | John Naylor | 2025-10-03 07:36:47 | Re: [PATCH] Hex-coding optimizations using SVE on ARM. |
Previous Message | Richard Guo | 2025-10-03 07:05:02 | Re: MergeAppend could consider sorting cheapest child path |