Horton

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

Masahiko Sawada <sawada.mshk@gmail.com>

View thread View on postgr.es
From: Masahiko Sawada <sawada.mshk@gmail.com>
To: Sutou Kouhei <kou@clear-code.com>
Cc: michael@paquier.xyz, zhjwpku@gmail.com, pgsql-hackers@postgresql.org
Date: 2025-02-26T01:14:43Z
Lists: pgsql-hackers

Attachments

On Tue, Feb 25, 2025 at 3:52 PM Sutou Kouhei <kou@clear-code.com> wrote:
>
>

Thank you for reviewing the patches. I've addressed comments except
for the following comment:

> > --- a/src/backend/commands/copyfromparse.c
> > +++ b/src/backend/commands/copyfromparse.c
>
> > @@ -1087,7 +1132,7 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
>
> >  static bool
> > -CopyReadLine(CopyFromState cstate)
> > +CopyReadLine(CopyFromState cstate, bool is_csv)
>
> > @@ -1163,7 +1208,7 @@ CopyReadLine(CopyFromState cstate)
>
> >  static bool
> > -CopyReadLineText(CopyFromState cstate)
> > +CopyReadLineText(CopyFromState cstate, bool is_csv)
>
> We may want to add a comment why we don't use "inline" nor
> "pg_attribute_always_inline" here:
>
> https://www.postgresql.org/message-id/CAD21AoBNfKDbJnu-zONNpG820ZXYC0fuTSLrJ-UdRqU4qp2wog%40mail.gmail.com
>
> > Yes, I'm not sure it's really necessary to make it inline since the
> > benchmark results don't show much difference. Probably this is because
> > the function has 'is_csv' in some 'if' branches but the compiler
> > cannot optimize out the whole 'if' branches as most 'if' branches
> > check 'is_csv' and other variables.
>
> Or we can add "inline" not "pg_attribute_always_inline" here
> as a hint for compiler.

I think we should not add inline unless we see a performance
improvement. Also, I find that it would be independent with this
refactoring so we can add it later if needed.

I've attached updated patches.

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

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.