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

From: Sutou Kouhei <kou(at)clear-code(dot)com>
To: zhjwpku(at)gmail(dot)com
Cc: sawada(dot)mshk(at)gmail(dot)com, andrew(at)dunslane(dot)net, michael(at)paquier(dot)xyz, 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-29 09:45:23
Message-ID: 20240129.184523.703743711014180020.kou@clear-code.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

In <CAEG8a3Jnmbjw82OiSvRK3v9XN2zSshsB8ew1mZCQDAkKq6r9YQ(at)mail(dot)gmail(dot)com>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Mon, 29 Jan 2024 11:37:07 +0800,
Junwang Zhao <zhjwpku(at)gmail(dot)com> wrote:

>> > > Does it make sense to pass only non-builtin options to the custom
>> > > format callback after parsing and evaluating the builtin options? That
>> > > is, we parse and evaluate only the builtin options and populate
>> > > opts_out first, then pass each rest option to the custom format
>> > > handler callback. The callback can refer to the builtin option values.
>>
>> What I imagined is that while parsing the all specified options, we
>> evaluate builtin options and we add non-builtin options to another
>> list. Then when parsing a non-builtin option, we check if this option
>> already exists in the list. If there is, we raise the "option %s not
>> recognized" error.". Once we complete checking all options, we pass
>> each option in the list to the callback.

I implemented this idea and the following ideas:

1. Add init callback for initialization
2. Change GetFormat() to FillCopyXXXResponse()
because JSON format always use 1 column
3. FROM only: Eliminate more cstate->opts.csv_mode branches
(This is for performance.)

See the attached v9 patch set for details. Changes since v7:

0001:

* Move CopyToProcessOption() calls to the end of
ProcessCopyOptions() for easy to option validation
* Add CopyToState::CopyToInit() and call it in
ProcessCopyOptionFormatTo()
* Change CopyToState::CopyToGetFormat() to
CopyToState::CopyToFillCopyOutResponse() and use it in
SendCopyBegin()

0002:

* Move CopyFromProcessOption() calls to the end of
ProcessCopyOptions() for easy to option validation
* Add CopyFromState::CopyFromInit() and call it in
ProcessCopyOptionFormatFrom()
* Change CopyFromState::CopyFromGetFormat() to
CopyFromState::CopyFromFillCopyOutResponse() and use it in
ReceiveCopyBegin()
* Rename NextCopyFromRawFields() to
NextCopyFromRawFieldsInternal() and pass the read
attributes callback explicitly to eliminate more
cstate->opts.csv_mode branches

Thanks,
--
kou

Attachment Content-Type Size
v9-0001-Extract-COPY-TO-format-implementations.patch text/x-patch 30.1 KB
v9-0002-Extract-COPY-FROM-format-implementations.patch text/x-patch 31.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Hayato Kuroda (Fujitsu) 2024-01-29 09:57:44 RE: speed up a logical replica setup
Previous Message Dean Rasheed 2024-01-29 09:44:21 Re: Supporting MERGE on updatable views