Re: Make COPY format extendable: Extract COPY TO format implementations

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

In response to

Browse pgsql-hackers by date

  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