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

Hi,

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

In <CAD21AoAXzwPC7jjPMTcT80hnzmPa2SUJkiqdYHweEY8sZscEMA(at)mail(dot)gmail(dot)com>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Wed, 23 Apr 2025 23:44:55 -0700,
Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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