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