Re: pgsql: Refactor COPY FROM to use format callback functions.

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