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

On Fri, Mar 8, 2024 at 8:23 AM Sutou Kouhei <kou(at)clear-code(dot)com> wrote:
>
>
> This shows that the v17 approach doesn't affect the current
> text/csv/binary implementations. (The v17 approach just adds
> 2 new structs, Copy{From,To}Rountine, without changing the
> current text/csv/binary implementations.)
>
> Can we push the v17 patch and proceed following
> implementations? Could someone (especially a PostgreSQL
> committer) take a look at this for double-check?
>

Hi, here are my cents:
Currently in v17, we have 3 extra functions within DoCopyTo
CopyToStart, one time, start, doing some preliminary work.
CopyToOneRow, doing the repetitive work, called many times, row by row.
CopyToEnd, one time doing the closing work.

seems to need a function pointer for processing the format and other options.
or maybe the reason is we need a one time function call before doing DoCopyTo,
like one time initialization.

We can placed the function pointer after:
`
cstate = BeginCopyTo(pstate, rel, query, relid,
stmt->filename, stmt->is_program,
NULL, stmt->attlist, stmt->options);
`

generally in v17, the code pattern looks like this.
if (cstate->opts.binary)
{
/* handle binary format */
}
else if (cstate->routine)
{
/* custom code, make the copy format extensible */
}
else
{
/* handle non-binary, (csv or text) format */
}
maybe we need another bool flag like `bool buildin_format`.
if the copy format is {csv|text|binary} then buildin_format is true else false.

so the code pattern would be:
if (cstate->opts.binary)
{
/* handle binary format */
}
else if (cstate->routine && !buildin_format)
{
/* custom code, make the copy format extensible */
}
else
{
/* handle non-binary, (csv or text) format */
}

otherwise the {CopyToRoutine| CopyFromRoutine} needs a function pointer
to distinguish native copy format and extensible supported format,
like I mentioned above?