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

On Sat, Mar 1, 2025 at 10:50 AM Sutou Kouhei <kou(at)clear-code(dot)com> wrote:
>
> Hi,
>
> Our 0001/0002 patches were merged into master. I've rebased
> on master. Can we discuss how to proceed rest patches?
>
> The contents of them aren't changed but I'll show a summary
> of them again:
>
> 0001-0003 are for COPY TO and 0004-0007 are for COPY FROM.
>
> For COPY TO:
>
> 0001: Add support for adding custom COPY TO format. This
> uses tablesample like handler approach. We've discussed
> other approaches such as USING+CREATE XXX approach but it
> seems that other approaches are overkill for this case.
>
> See also: https://www.postgresql.org/message-id/flat/d838025aceeb19c9ff1db702fa55cabf%40postgrespro.ru#caca2799effc859f82f40ee8bec531d8
>
> 0002: Export CopyToStateData to implement custom COPY TO
> format as extension.
>
> 0003: Export a function and add a private space to
> CopyToStateData to implement custom COPY TO format as
> extension.
>
> We may want to squash 0002 and 0003 but splitting them will
> be easy to review. Because 0002 just moves existing codes
> (with some rename) and 0003 just adds some codes. If we
> squash 0002 and 0003, moving and adding are mixed.
>
> For COPY FROM:
>
> 0004: This is COPY FROM version of 0001.
>
> 0005: 0002 has COPY_ prefix -> COPY_DEST_ prefix change for
> enum CopyDest. This is similar change for enum CopySource.
>
> 0006: This is COPY FROM version of 0003.
>
> 0007: This is for easy to implement "ON_ERROR stop" and
> "LOG_VERBOSITY verbose" in extension.
>
> We may want to squash 0005-0007 like for 0002-0003.
>
>
> Thanks,
> --
> kou

While review another thread (Emitting JSON to file using COPY TO),
I found the recently committed patches on this thread pass the
CopyFormatOptions struct directly rather a pointer of the struct
as a function parameter of CopyToGetRoutine and CopyFromGetRoutine.

Then I took a quick look at the newly rebased patch set and
found Sutou has already fixed this issue.

I'm wondering if we should fix it as a separate commit as
it seems like an oversight of previous patches?

--
Regards
Junwang Zhao