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: sawada.mshk@gmail.com, zhjwpku@gmail.com, andrew@dunslane.net, nathandbossart@gmail.com, pgsql-hackers@postgresql.org
Date: 2023-12-22T01:00:24Z
Lists: pgsql-hackers 
On Thu, Dec 21, 2023 at 06:35:04PM +0900, Sutou Kouhei wrote:
>    * If we just require "copy_to_${FORMAT}(internal)"
>      function and "copy_from_${FORMAT}(internal)" function,
>      we can remove the tricky approach. And it also avoid
>      name collisions with other handler such as tablesample
>      handler.
>      See also:
>      https://www.postgresql.org/message-id/flat/20231214.184414.2179134502876898942.kou%40clear-code.com#af71f364d0a9f5c144e45b447e5c16c9

Hmm.  I prefer the unique name approach for the COPY portions without
enforcing any naming policy on the function names returning the
handlers, actually, though I can see your point.

> 2. Need an opaque space like IndexScanDesc::opaque does
> 
>    * A custom COPY TO handler needs to keep its data

Sounds useful to me to have a private area passed down to the
callbacks.

> 3. Export CopySend*()
> 
>    * If we like minimum API, we just need to export
>      CopySendData() and CopySendEndOfRow(). But
>      CopySend{String,Char,Int32,Int16}() will be convenient
>      custom COPY TO handlers. (A custom COPY TO handler for
>      Apache Arrow doesn't need them.)

Hmm.  Not sure on this one.  This may come down to externalize the
manipulation of fe_msgbuf.  Particularly, could it be possible that
some custom formats don't care at all about the network order?

> Questions:
> 
> 1. What value should be used for "format" in
>    PgMsg_CopyOutResponse message?
> 
>    It's 1 for binary format and 0 for text/csv format.
> 
>    Should we make it customizable by custom COPY TO handler?
>    If so, what value should be used for this?

Interesting point.  It looks very tempting to give more flexibility to
people who'd like to use their own code as we have one byte in the
protocol but just use 0/1.  Hence it feels natural to have a callback
for that.

It also means that we may want to think harder about copy_is_binary in
libpq in the future step.  Now, having a backend implementation does
not need any libpq bits, either, because a client stack may just want
to speak the Postgres protocol directly.  Perhaps a custom COPY
implementation would be OK with how things are in libpq, as well,
tweaking its way through with just text or binary.

> 2. Do we need more tries for design discussion for the first
>    implementation? If we need, what should we try?

A makeNode() is used with an allocation in the current memory context
in the function returning the handler.  I would have assume that this
stuff returns a handler as a const struct like table AMs.
--
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.