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

Sutou Kouhei <kou@clear-code.com>

From: Sutou Kouhei <kou@clear-code.com>
To: sawada.mshk@gmail.com
Cc: david.g.johnston@gmail.com, tgl@sss.pgh.pa.us, zhjwpku@gmail.com, michael@paquier.xyz, pgsql-hackers@postgresql.org
Date: 2025-04-25T12:45:34Z
Lists: pgsql-hackers

Attachments

Hi,

I've updated the patch set. See the attached v40 patch set.

In <CAD21AoAXzwPC7jjPMTcT80hnzmPa2SUJkiqdYHweEY8sZscEMA@mail.gmail.com>
  "Re: Make COPY format extendable: Extract COPY TO format implementations" on Wed, 23 Apr 2025 23:44:55 -0700,
  Masahiko Sawada <sawada.mshk@gmail.com> wrote:

>> Are the followings correct?
>>
>> 1. Move invalid input patterns in
>>    src/test/modules/test_copy_format/sql/invalid.sql to
>>    src/test/regress/sql/copy.sql as much as possible.
>> 2. Create
>>    src/test/modules/test_copy_format/sql/test_copy_format.sql
>>    and move all contents in existing *.sql to the file.
>> 3. Add comments what the tests expect to
>>    src/test/modules/test_copy_format/sql/test_copy_format.sql.
>> 4. Remove CopyFormatOptions::{binary,csv_mode}.
> 
> Agreed with the above items.

Done except 1. because 1. is removed by 3. in the following
list:

----
>> There are 3 unconfirmed suggested changes for tests in:
>> https://www.postgresql.org/message-id/20250330.113126.433742864258096312.kou%40clear-code.com
>>
>> Here are my opinions for them:
>>
>> > 1.: There is no difference between single-quoting and
>> >     double-quoting here. Because the information what quote
>> >     was used for the given FORMAT value isn't remained
>> >     here. Should we update gram.y?
>> >
>> > 2.: I don't have a strong opinion for it. If nobody objects
>> >     it, I'll remove them.
>> >
>> > 3.: I don't have a strong opinion for it. If nobody objects
>> >     it, I'll remove them.
----

0005 is added for 4. Could you squash 0004 ("Use copy
handler for bult-in formats") and 0005 ("Remove
CopyFormatOptions::{binary,csv_mode}") if needed when you
push?

>> 6. Use handler OID for detecting the default built-in format
>>    instead of comparing the given format as string.

Done.

>> 7. Update documentation.

Could someone help this? 0007 is the draft commit for this.

>> There are 3 unconfirmed suggested changes for tests in:
>> https://www.postgresql.org/message-id/20250330.113126.433742864258096312.kou%40clear-code.com
>>
>> Here are my opinions for them:
>>
>> > 1.: There is no difference between single-quoting and
>> >     double-quoting here. Because the information what quote
>> >     was used for the given FORMAT value isn't remained
>> >     here. Should we update gram.y?
>> >
>> > 2.: I don't have a strong opinion for it. If nobody objects
>> >     it, I'll remove them.
>> >
>> > 3.: I don't have a strong opinion for it. If nobody objects
>> >     it, I'll remove them.
>>
>> Is the 1. required for "ready for merge"? If so, is there
>> any suggestion? I don't have a strong opinion for it.
>>
>> If there are no more opinions for 2. and 3., I'll remove
>> them.
> 
> Agreed.

1.: I didn't do anything. Because there is no suggestion.

2., 3.: Done.

> I think we would still need some rounds of reviews but the patch is
> getting in good shape.

I hope that this is completed in this year...


Thanks,
-- 
kou

Commits

  1. Refactor Copy{From|To}GetRoutine() to use pass-by-reference argument.

  2. Refactor COPY FROM to use format callback functions.

  3. Refactor COPY TO to use format callback functions.

  4. Another try to fix BF failure introduced in commit ddd5f4f54a.

  5. Revert "Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM"

  6. Improve COPY TO performance when server and client encodings match

  7. Simplify signature of CopyAttributeOutCSV() in copyto.c

  8. Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"

  9. Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO

  10. Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM

  11. Add progress reporting of skipped tuples during COPY FROM.

  12. pgbench: Add \syncpipeline

  13. meson: Make gzip and tar optional

  14. Export the external file reader used in COPY FROM as APIs.