From: Sutou Kouhei <kou@clear-code.com>
To: sawada.mshk@gmail.com
Cc: andrew@dunslane.net
Cc: msawada@postgresql.org
Cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Refactor COPY FROM to use format callback functions.
Date: Sat, 01 Mar 2025 07:16:41 +0900 (JST)
Message-ID: <20250301.071641.1257013931056303227.kou@clear-code.com> (raw)
In-Reply-To: <CAD21AoBA414Q76LthY65NJfWbjOxXn1bdFFsD_NBhT2wPUS1SQ@mail.gmail.com>
References: <CAD21AoDrDQy-cBHaghYkFb=_iPhgJg0H-hscrRtBf3r+xaVe-Q@mail.gmail.com>
<d3b11749-7d0d-40c6-b2be-0e45958881da@dunslane.net>
<CAD21AoBA414Q76LthY65NJfWbjOxXn1bdFFsD_NBhT2wPUS1SQ@mail.gmail.com>
Hi,
Thanks for following up the patch!
In <CAD21AoBA414Q76LthY65NJfWbjOxXn1bdFFsD_NBhT2wPUS1SQ@mail.gmail.com>
"Re: pgsql: Refactor COPY FROM to use format callback functions." on Fri, 28 Feb 2025 12:56:19 -0800,
Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> Right. I've attached the updated patch.
In general, this approach will work but could you keep
the same signature for backward compatibility?
> --- a/src/backend/commands/copyfromparse.c
> +++ b/src/backend/commands/copyfromparse.c
> +bool
> +NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields, bool is_csv)
> +{
> + return NextCopyFromRawFieldsInternal(cstate, fields, nfields, is_csv);
> +}
NextCopyFromRawFields() uses
NextCopyFromRawFields(CopyFromState cstate, char ***fields,
int *nfields) (no "bool is_csv") before we remove it. So
could you use the no "bool is_csv" signature for backward
compatibility?
bool
NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
{
return NextCopyFromRawFieldsInternal(cstate, fields, nfields, cstate->opts.csv_mode);
}
> @@ -738,6 +742,15 @@ CopyReadBinaryData(CopyFromState cstate, char *dest, int nbytes)
> /*
> * Read raw fields in the next line for COPY FROM in text or csv mode.
> * Return false if no more lines.
> + */
> +bool
> +NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields, bool is_csv)
> +{
> + return NextCopyFromRawFieldsInternal(cstate, fields, nfields, is_csv);
> +}
> +
> +/*
> + * Workhorse for NextCopyFromRawFields().
It may be better that we don't split docs for
NextCopyFromRawFields() and
NextCopyFromRawFieldsInternal(). How about referring the
NextCopyFromRawFieldsInternal() doc from the
NextCopyFromRawFields() doc something like the following?
/*
* See NextCopyFromRawFieldsInternal() for details.
*/
bool
NextCopyFromRawFields(...)
{
...
}
/*
* Workhorse for NextCopyFromRawFields().
*
* Read raw fields ...
*
* ...
*/
static pg_attribute_always_inline bool
NextCopyFromRawFieldsInternal()
{
...
}
Thanks,
--
kou
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: kou@clear-code.com, sawada.mshk@gmail.com, andrew@dunslane.net, msawada@postgresql.org, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Refactor COPY FROM to use format callback functions.
In-Reply-To: <20250301.071641.1257013931056303227.kou@clear-code.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox