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

Sutou Kouhei <kou@clear-code.com>

From: Sutou Kouhei <kou@clear-code.com>
To: sawada.mshk@gmail.com
Cc: michael@paquier.xyz, zhjwpku@gmail.com, andrew@dunslane.net, nathandbossart@gmail.com, pgsql-hackers@postgresql.org
Date: 2024-01-15T06:27:02Z
Lists: pgsql-hackers
Hi,

If there are no more comments for the current design, I'll
start implementing this feature with the following
approaches for "Discussing" items:

> 3.1 Should we use one function(internal) for COPY TO/FROM
>     handlers or two function(internal)s (one is for COPY TO
>     handler and another is for COPY FROM handler)?
>     [4]

I'll choose "one function(internal) for COPY TO/FROM handlers".

> 3.4 Should we export Copy{To,From}State? Or should we just
>     provide getters/setters to access Copy{To,From}State
>     internal?
>     [10]

I'll export Copy{To,From}State.


Thanks,
-- 
kou

In <20240112.144615.157925223373344229.kou@clear-code.com>
  "Re: Make COPY format extendable: Extract COPY TO format implementations" on Fri, 12 Jan 2024 14:46:15 +0900 (JST),
  Sutou Kouhei <kou@clear-code.com> wrote:

> Hi,
> 
> Here is the current summary for a this discussion to make
> COPY format extendable. It's for reaching consensus and
> starting implementing the feature. (I'll start implementing
> the feature once we reach consensus.) If you have any
> opinion, please share it.
> 
> Confirmed:
> 
> 1.1 Making COPY format extendable will not reduce performance.
>     [1]
> 
> Decisions:
> 
> 2.1 Use separated handler for COPY TO and COPY FROM because
>     our COPY TO implementation (copyto.c) and COPY FROM
>     implementation (coypfrom.c) are separated.
>     [2]
> 
> 2.2 Don't use system catalog for COPY TO/FROM handlers. We can
>     just use a function(internal) that returns a handler instead.
>     [3]
> 
> 2.3 The implementation must include documentation.
>     [5]
> 
> 2.4 The implementation must include test.
>     [6]
> 
> 2.5 The implementation should be consist of small patches
>     for easy to review.
>     [6]
> 
> 2.7 Copy{To,From}State must have a opaque space for
>     handlers.
>     [8]
> 
> 2.8 Export CopySendData() and CopySendEndOfRow() for COPY TO
>     handlers.
>     [8]
> 
> 2.9 Make "format" in PgMsg_CopyOutResponse message
>     extendable.
>     [9]
> 
> 2.10 Make makeNode() call avoidable in function(internal)
>      that returns COPY TO/FROM handler.
>      [9]
> 
> 2.11 Custom COPY TO/FROM handlers must be able to parse
>      their options.
>      [11]
> 
> Discussing:
> 
> 3.1 Should we use one function(internal) for COPY TO/FROM
>     handlers or two function(internal)s (one is for COPY TO
>     handler and another is for COPY FROM handler)?
>     [4]
> 
> 3.2 If we use separated function(internal) for COPY TO/FROM
>     handlers, we need to define naming rule. For example,
>     <method_name>_to(internal) for COPY TO handler and
>     <method_name>_from(internal) for COPY FROM handler.
>     [4]
> 
> 3.3 Should we use prefix or suffix for function(internal)
>     name to avoid name conflict with other handlers such as
>     tablesample handlers?
>     [7]
> 
> 3.4 Should we export Copy{To,From}State? Or should we just
>     provide getters/setters to access Copy{To,From}State
>     internal?
>     [10]
> 
> 
> [1] https://www.postgresql.org/message-id/flat/20231204.153548.2126325458835528809.kou%40clear-code.com
> [2] https://www.postgresql.org/message-id/flat/ZXEUIy6wl4jHy6Nm%40paquier.xyz
> [3] https://www.postgresql.org/message-id/flat/CAD21AoAhcZkAp_WDJ4sSv_%2Bg2iCGjfyMFgeu7MxjnjX_FutZAg%40mail.gmail.com
> [4] https://www.postgresql.org/message-id/flat/CAD21AoDkoGL6yJ_HjNOg9cU%3DaAdW8uQ3rSQOeRS0SX85LPPNwQ%40mail.gmail.com
> [5] https://www.postgresql.org/message-id/flat/TY3PR01MB9889C9234CD220A3A7075F0DF589A%40TY3PR01MB9889.jpnprd01.prod.outlook.com
> [6] https://www.postgresql.org/message-id/flat/ZXbiPNriHHyUrcTF%40paquier.xyz
> [7] https://www.postgresql.org/message-id/flat/20231214.184414.2179134502876898942.kou%40clear-code.com
> [8] https://www.postgresql.org/message-id/flat/20231221.183504.1240642084042888377.kou%40clear-code.com
> [9] https://www.postgresql.org/message-id/flat/ZYTfqGppMc9e_w2k%40paquier.xyz
> [10] https://www.postgresql.org/message-id/flat/CAD21AoD%3DUapH4Wh06G6H5XAzPJ0iJg9YcW8r7E2UEJkZ8QsosA%40mail.gmail.com
> [11] https://www.postgresql.org/message-id/flat/20240110.152023.1920937326588672387.kou%40clear-code.com
> 
> 
> Thanks,
> -- 
> kou
> 
> 



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.