Hi, In <CAD21AoC=DX5QQVb27C6UdpPfY-F=-PGnQ1u6rWo69DV=4et...@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.m...@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
