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

From: Sutou Kouhei <kou(at)clear-code(dot)com>
To: michael(at)paquier(dot)xyz
Cc: andrew(at)dunslane(dot)net, sawada(dot)mshk(at)gmail(dot)com, zhjwpku(at)gmail(dot)com, nathandbossart(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
Date: 2024-01-26 08:49:47
Message-ID: 20240126.174947.1394046405836416758.kou@clear-code.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

In <ZbLwNyOKbddno0Ue(at)paquier(dot)xyz>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Fri, 26 Jan 2024 08:35:19 +0900,
Michael Paquier <michael(at)paquier(dot)xyz> wrote:

>> OK. How about the following for the fetch function
>> signature?
>>
>> extern CopyToRoutine *GetBuiltinCopyToRoutine(const char *format);
>
> Or CopyToRoutineGet()? I am not wedded to my suggestion, got a bad
> history with naming things around here.

Thanks for the suggestion.
I rethink about this and use the following:

+extern void ProcessCopyOptionFormatTo(ParseState *pstate, CopyFormatOptions *opts_out, DefElem *defel);

It's not a fetch function. It sets CopyToRoutine opts_out
instead. But it hides CopyToRoutine* to copyto.c. Is it
acceptable?

>> OK. I'll focus on 0001 and 0005 for now. I'll restart
>> 0002-0004/0006-0008 after 0001 and 0005 are accepted.
>
> Once you get these, I'd be interested in re-doing an evaluation of
> COPY TO and more tests with COPY FROM while running Postgres on
> scissors. One thing I was thinking to use here is my blackhole_am for
> COPY FROM:
> https://github.com/michaelpq/pg_plugins/tree/main/blackhole_am

Thanks!

Could you evaluate the attached patch set with COPY FROM?

I attach v7 patch set. It includes only the 0001 and 0005
parts in v6 patch set because we focus on them for now.

0001: This is based on 0001 in v6.

Changes since v6:

* Fix comment style
* Hide CopyToRoutine{Text,CSV,Binary}
* Add more comments
* Eliminate "if (cstate->opts.csv_mode)" branches from "text"
and "csv" callbacks
* Remove CopyTo*_function typedefs
* Update benchmark results in commit message but the results
are measured on my environment that isn't suitable for
accurate benchmark

0002: This is based on 0005 in v6.

Changes since v6:

* Fix comment style
* Hide CopyFromRoutine{Text,CSV,Binary}
* Add more comments
* Eliminate a "if (cstate->opts.csv_mode)" branch from "text"
and "csv" callbacks
* NOTE: We can eliminate more "if (cstate->opts.csv_mode)" branches
such as one in NextCopyFromRawFields(). Should we do it
in this feature improvement (make COPY format
extendable)? Can we defer this as a separated improvement?
* Remove CopyFrom*_function typedefs

Thanks,
--
kou

Attachment Content-Type Size
v7-0001-Extract-COPY-TO-format-implementations.patch text/x-patch 27.7 KB
v7-0002-Extract-COPY-FROM-format-implementations.patch text/x-patch 27.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Sutou Kouhei 2024-01-26 08:55:11 Re: Make COPY format extendable: Extract COPY TO format implementations
Previous Message Junwang Zhao 2024-01-26 08:41:50 Re: Make COPY format extendable: Extract COPY TO format implementations