Horton

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

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: Sutou Kouhei <kou@clear-code.com>
Cc: andres@anarazel.de, sawada.mshk@gmail.com, zhjwpku@gmail.com, andrew@dunslane.net, nathandbossart@gmail.com, pgsql-hackers@postgresql.org
Date: 2024-02-09T08:25:50Z
Lists: pgsql-hackers 
On Fri, Feb 09, 2024 at 04:32:05PM +0900, Sutou Kouhei wrote:
> In <ZcWoTr1N0GELFA9E@paquier.xyz>
>   "Re: Make COPY format extendable: Extract COPY TO format implementations" on Fri, 9 Feb 2024 13:21:34 +0900,
>   Michael Paquier <michael@paquier.xyz> wrote:
>> A next step I think we could take is to split the binary-only and the
>> text/csv-only data in each cstate into their own structure to make the
>> structure, with an opaque pointer that custom formats could use, but a
>> lot of fields are shared as well.
> 
> It'll make COPY code base cleaner but it may decrease
> performance.

Perhaps, but I'm not sure, TBH.  But perhaps others can comment on
this point.  This surely needs to be studied closely.

> My suggestion:
> 1. Introduce Copy{To,From}Routine
>    (We can do it based on the v14 patch.)
> 2. Add an opaque pointer to Copy{To,From}Routine
>    (This must not have performance impact.)
> 3.a. Split format specific data to the opaque space
> 3.b. Add support for registering custom format handler by
>      creating a function
> 4. ...

4. is going to need 3.  At this point 3.b sounds like the main thing
to tackle first if we want to get something usable for the end-user
into this release, at least.  Still 2 is important for pluggability
as we pass the cstates across all the routines and custom formats want
to save their own data, so this split sounds OK.  I am not sure how
much of 3.a we really need to do for the in-core formats.

> I think that we should not use this approach for
> performance. We need to use "static inline" and constant
> argument instead something like the attached
> remove-copy-read-attributes.diff.

FWIW, using inlining did not show any performance change here.
Perhaps that's only because this is called in the COPY FROM path once
per row (even for the case of using 1 attribute with blackhole_am).
--
Michael

Commits

  1. Refactor Copy{From|To}GetRoutine() to use pass-by-reference argument.

  2. Refactor COPY FROM to use format callback functions.

  3. Refactor COPY TO to use format callback functions.

  4. Another try to fix BF failure introduced in commit ddd5f4f54a.

  5. Revert "Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM"

  6. Improve COPY TO performance when server and client encodings match

  7. Simplify signature of CopyAttributeOutCSV() in copyto.c

  8. Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"

  9. Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO

  10. Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM

  11. Add progress reporting of skipped tuples during COPY FROM.

  12. pgbench: Add \syncpipeline

  13. meson: Make gzip and tar optional

  14. Export the external file reader used in COPY FROM as APIs.