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

On Tue, Jul 15, 2025 at 5:37 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2025-07-14 03:28:16 +0900, Masahiko Sawada wrote:
> > I've reviewed the 0001 and 0002 patches. The API implemented in the
> > 0002 patch looks good to me, but I'm concerned about the capsulation
> > of copy state data. With the v42 patches, we pass the whole
> > CopyToStateData to the extension codes, but most of the fields in
> > CopyToStateData are internal working state data that shouldn't be
> > exposed to extensions. I think we need to sort out which fields are
> > exposed or not. That way, it would be safer and we would be able to
> > avoid exposing copyto_internal.h and extensions would not need to
> > include copyfrom_internal.h.
> >
> > I've implemented a draft patch for that idea. In the 0001 patch, I
> > moved fields that are related to internal working state from
> > CopyToStateData to CopyToExectuionData. COPY routine APIs pass a
> > pointer of CopyToStateData but extensions can access only fields
> > except for CopyToExectuionData. In the 0002 patch, I've implemented
> > the registration API and some related APIs based on your v42 patch.
> > I've made similar changes to COPY FROM codes too.
>
> I've not followed the development of this patch - but I continue to be
> concerned about the performance impact it has as-is and the amount of COPY
> performance improvements it forecloses.
>
> This seems to add yet another layer of indirection to a lot of hot functions
> like CopyGetData() etc.
>

The most refactoring works have been done by commit 7717f6300 and
2e4127b6d with a slight performance gain. At this stage, we're trying
to introduce the registration API so that extensions can provide their
callbacks to the core. Some functions required for I/O such as
CopyGetData() and CopySendEndOfRow() would be exposed but I'm not
going to add additional indirection function call layers.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com