Horton

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

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Michael Paquier <michael@paquier.xyz>
Cc: Sutou Kouhei <kou@clear-code.com>, sawada.mshk@gmail.com, zhjwpku@gmail.com, andrew@dunslane.net, nathandbossart@gmail.com, pgsql-hackers@postgresql.org
Date: 2024-02-06T23:33:36Z
Lists: pgsql-hackers

Attachments

Hi,

On 2024-02-06 15:11:05 +0900, Michael Paquier wrote:
> On Mon, Feb 05, 2024 at 09:46:42PM -0800, Andres Freund wrote:
> > No - what I mean is that it doesn't make sense to have copy_attribute_out(),
> > as e.g. CopyToTextOneRow() already knows that it's dealing with text, so it
> > can directly call the right function. That does require splitting a bit more
> > between csv and text output, but I think that can be done without much
> > duplication.
> 
> I am not sure to understand here.  In what is that different from
> reverting 2889fd23be56 then mark CopyAttributeOutCSV and
> CopyAttributeOutText as static inline?

Well, you can't just do that, because there's only one caller, namely
CopyToTextOneRow(). What I am trying to suggest is something like the
attached, just a quick hacky POC. Namely to split out CSV support from
CopyToTextOneRow() by introducing CopyToCSVOneRow(), and to avoid code
duplication by moving the code into a new CopyToTextLikeOneRow().

I named it CopyToTextLike* here, because it seems confusing that some Text*
are used for both CSV and text and others are actually just for text. But if
were to go for that, we should go further.


To test the performnce effects I chose to remove the pointless encoding
"check" we're discussing in the other thread, as it makes it harder to see the
time differences due to the per-attribute code.  I did three runs of pgbench
-t of [1] and chose the fastest result for each.


With turbo mode and power saving disabled:

                          Avg Time
HEAD                       995.349
Remove Encoding Check      870.793
v13-0001                   869.678
Remove out callback        839.508

Greetings,

Andres Freund

[1] COPY (SELECT 1::int2,2::int2,3::int2,4::int2,5::int2,6::int2,7::int2,8::int2,9::int2,10::int2,11::int2,12::int2,13::int2,14::int2,15::int2,16::int2,17::int2,18::int2,19::int2,20::int2, generate_series(1, 1000000::int4)) TO '/dev/null';

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.