Horton

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

Junwang Zhao <zhjwpku@gmail.com>

View thread View on postgr.es
From: Junwang Zhao <zhjwpku@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Sutou Kouhei <kou@clear-code.com>, nathandbossart@gmail.com, pgsql-hackers@postgresql.org
Date: 2023-12-07T08:37:36Z
Lists: pgsql-hackers 
On Thu, Dec 7, 2023 at 8:39 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> 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,

The point of this refactor (from my view) is to make it possible to add new
copy handlers in extensions, just like access method. As Andres suggested,
a system catalog like *pg_copy_handler*, if we split TO and FROM into two
sets of routines, does that mean we have to create two catalog(
pg_copy_from_handler and pg_copy_to_handler)?

> 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



-- 
Regards
Junwang Zhao

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.