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

From: Sutou Kouhei <kou(at)clear-code(dot)com>
To: sawada(dot)mshk(at)gmail(dot)com
Cc: michael(at)paquier(dot)xyz, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
Date: 2024-11-19 01:31:15
Message-ID: 20241119.103115.418773618386970482.kou@clear-code.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

In <CAD21AoC=DX5QQVb27C6UdpPfY-F=-PGnQ1u6rWo69DV=4EtDdw(at)mail(dot)gmail(dot)com>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Mon, 18 Nov 2024 17:02:41 -0800,
Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2024-11-19 01:42:35 Re: Statistics Import and Export
Previous Message Corey Huinker 2024-11-19 01:29:10 Re: Statistics Import and Export