On Fri, Oct 3, 2025 at 12:06 AM Sutou Kouhei <kou@clear-code.com> wrote:
>
> Hi,
>
> In <CAD21AoADXWgdizS0mV5w8wdfftDRsm8sUtNW=CzYYS1OhjFD2A@mail.gmail.com>
> "Re: Make COPY format extendable: Extract COPY TO format implementations" on Mon, 15 Sep 2025 10:00:18 -0700,
> Masahiko Sawada <sawada.mshk@gmail.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.
Thank you for drafting the idea!
The patch refactors the CopyToStateData so that we can both hide
internal-use-only fields from extensions and extension can use its own
state data, while not adding extra indirection layers. TBH I'm really
not sure we must fully hide internal fields from extensions. Other
extendable components seem not to strictly hide internal information
from extensions. I'd suggest starting with only the latter point. That
is, we merge fields in CopyToStateInternalData to CopyToStateData.
What do you think?
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
