Re: Make COPY format extendable: Extract COPY TO format implementations
Masahiko Sawada <sawada.mshk@gmail.com>
Attachments
- 0001-Refactor-CopyToStateData-and-CopyFromStateData.patch (application/octet-stream) patch 0001
- 0002-Custom-COPY-format.patch (application/octet-stream) patch 0002
On Mon, Jun 30, 2025 at 3:00 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Wed, Jun 25, 2025 at 4:35 PM Sutou Kouhei <kou@clear-code.com> wrote:
> >
> > Hi,
> >
> > In <CAD21AoC19fV5Ujs-1r24MNU+hwTQUeZMEnaJDjSFwHLMMdFi0Q@mail.gmail.com>
> > "Re: Make COPY format extendable: Extract COPY TO format implementations" on Wed, 25 Jun 2025 00:48:46 +0900,
> > Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > >> >> It's natural to add more related APIs with this
> > >> >> approach. The single registration API provides one feature
> > >> >> by one operation. If we use the RegisterCopyRoutine() for
> > >> >> FROM and TO formats API, it's not natural that we add more
> > >> >> related APIs. In this case, some APIs may provide multiple
> > >> >> features by one operation and other APIs may provide single
> > >> >> feature by one operation. Developers may be confused with
> > >> >> the API. For example, developers may think "what does mean
> > >> >> NULL here?" or "can we use NULL here?" for
> > >> >> "RegisterCopyRoutine("new-format", NewFormatFromRoutine,
> > >> >> NULL)".
> > >> >
> > >> > We can document it in the comment for the registration function.
> > >>
> > >> I think that API that can be understandable without the
> > >> additional note is better API than API that needs some
> > >> notes.
> > >
> > > I don't see much difference in this case.
> >
> > OK. It seems that we can't agree on which API is better.
> >
> > I've implemented your idea as the v42 patch set. Can we
> > proceed this proposal with this approach? What is the next
> > step?
>
> I'll review the patches.
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.
The patch is a very PoC phase and we would need to scrutinize the
fields that should or should not be exposed. Feedback is very welcome.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Commits
-
Refactor Copy{From|To}GetRoutine() to use pass-by-reference argument.
- bacbc4863b3b 18.0 landed
-
Refactor COPY FROM to use format callback functions.
- 7717f6300693 18.0 landed
-
Refactor COPY TO to use format callback functions.
- 2e4127b6d2d8 18.0 landed
-
Another try to fix BF failure introduced in commit ddd5f4f54a.
- 9bc1eee988c3 17.0 cited
-
Revert "Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM"
- 06bd311bce24 17.0 landed
-
Improve COPY TO performance when server and client encodings match
- b619852086ed 17.0 cited
-
Simplify signature of CopyAttributeOutCSV() in copyto.c
- b9d6038d7048 17.0 landed
-
Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"
- 1aa8324b81fa 17.0 landed
-
Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO
- 2889fd23be56 17.0 landed
-
Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM
- 95fb5b49024a 17.0 landed
-
Add progress reporting of skipped tuples during COPY FROM.
- 729439607ad2 17.0 cited
-
pgbench: Add \syncpipeline
- 94edfe250c6a 17.0 cited
-
meson: Make gzip and tar optional
- 9ca6e7b9411e 17.0 cited
-
Export the external file reader used in COPY FROM as APIs.
- 8ddc05fb01ee 9.1.0 cited