Horton

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

Sutou Kouhei <kou@clear-code.com>

View thread View on postgr.es
From: Sutou Kouhei <kou@clear-code.com>
To: sawada.mshk@gmail.com
Cc: michael@paquier.xyz, pgsql-hackers@postgresql.org
Date: 2024-11-19T01:31:15Z
Lists: pgsql-hackers 
Hi,

In <CAD21AoC=DX5QQVb27C6UdpPfY-F=-PGnQ1u6rWo69DV=4EtDdw@mail.gmail.com>
  "Re: Make COPY format extendable: Extract COPY TO format implementations" on Mon, 18 Nov 2024 17:02:41 -0800,
  Masahiko Sawada <sawada.mshk@gmail.com> wrote:

> I have a question about v22. We use pg_attribute_always_inline for
> some functions to avoid function call overheads. Applying it to
> CopyToTextLikeOneRow() and CopyFromTextLikeOneRow() are legitimate as
> we've discussed. But there are more function where the patch applied
> it to:
> 
> -bool
> -NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
> +static pg_attribute_always_inline bool
> +NextCopyFromRawFields(CopyFromState cstate, char ***fields, int
> *nfields, bool is_csv)
> 
> -static bool
> -CopyReadLineText(CopyFromState cstate)
> +static pg_attribute_always_inline bool
> +CopyReadLineText(CopyFromState cstate, bool is_csv)
> 
> +static pg_attribute_always_inline void
> +CopyToTextLikeSendEndOfRow(CopyToState cstate)
> 
> I think it's out of scope of this patch even if these changes are
> legitimate. Is there any reason for these changes?

Yes for NextCopyFromRawFields() and CopyReadLineText().
No for CopyToTextLikeSendEndOfRow().

NextCopyFromRawFields() and CopyReadLineText() have "bool
is_csv". So I think that we should use
pg_attribute_always_inline (or inline) like
CopyToTextLikeOneRow() and CopyFromTextLikeOneRow(). I think
that it's not out of scope of this patch because it's a part
of CopyToTextLikeOneRow() and CopyFromTextLikeOneRow()
optimization.

Note: The optimization is based on "bool is_csv" parameter
and constant "true"/"false" argument function call. If we
can inline this function call, all "if (is_csv)" checks in
the function are removed.

pg_attribute_always_inline (or inline) for
CopyToTextLikeSendEndOfRow() is out of scope of this
patch. You're right.

I think that inlining CopyToTextLikeSendEndOfRow() is better
because it's called per row. But it's not related to the
optimization.


Should I create a new patch set without
pg_attribute_always_inline/inline for
CopyToTextLikeSendEndOfRow()? Or could you remove it when
you push?


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.