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: Junwang Zhao <zhjwpku@gmail.com>
Cc: Sutou Kouhei <kou@clear-code.com>, nathandbossart@gmail.com, pgsql-hackers@postgresql.org
Date: 2023-12-07T00:38:59Z
Lists: pgsql-hackers 
On Wed, Dec 06, 2023 at 10:07:51PM +0800, Junwang Zhao wrote:
> I read the thread[1] you posted and I think Andres's suggestion sounds great.
> 
> Should we extract both *copy to* and *copy from* for the first step, in that
> case we can add the pg_copy_handler catalog smoothly later.
> 
> Attached V4 adds 'extract copy from' and it passed the cirrus ci,
> please take a look.
> 
> I added a hook *copy_from_end* but this might be removed later if not used.
> 
> [1]: https://www.postgresql.org/message-id/20180211211235.5x3jywe5z3lkgcsr%40alap3.anarazel.de

I was looking at the differences between v3 posted by Sutou-san and
v4 from you, seeing that:

+/* Routines for a COPY HANDLER implementation. */
+typedef struct CopyHandlerOps
 {
     /* Called when COPY TO is started. This will send a header. */
-    void        (*start) (CopyToState cstate, TupleDesc tupDesc);
+    void        (*copy_to_start) (CopyToState cstate, TupleDesc tupDesc);
 
     /* Copy one row for COPY TO. */
-    void        (*one_row) (CopyToState cstate, TupleTableSlot *slot);
+    void        (*copy_to_one_row) (CopyToState cstate, TupleTableSlot *slot);
 
     /* Called when COPY TO is ended. This will send a trailer. */
-    void        (*end) (CopyToState cstate);
-} CopyToFormatOps;
+    void        (*copy_to_end) (CopyToState cstate);
+
+    void        (*copy_from_start) (CopyFromState cstate, TupleDesc tupDesc);
+    bool        (*copy_from_next) (CopyFromState cstate, ExprContext *econtext,
+                                    Datum *values, bool *nulls);
+    void        (*copy_from_error_callback) (CopyFromState cstate);
+    void        (*copy_from_end) (CopyFromState cstate);
+} CopyHandlerOps;

And we've spent a good deal of time refactoring the copy code so as
the logic behind TO and FROM is split.  Having a set of routines that
groups both does not look like a step in the right direction to me,
and v4 is an attempt at solving two problems, while v3 aims to improve
one case.  It seems to me that each callback portion should be focused
on staying in its own area of the code, aka copyfrom*.c or copyto*.c.
--
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.