Re: Make COPY format extendable: Extract COPY TO format implementations
Sutou Kouhei <kou@clear-code.com>
Attachments
- v40-0001-Export-CopyDest-as-private-data.patch (text/x-patch)
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
-
Refactor Copy{From|To}GetRoutine() to use pass-by-reference argument.
- bacbc4863b3b 18.0 landed
-
Refactor COPY FROM to use format callback functions.
- 7717f6300693 18.0 landed
-
Refactor COPY TO to use format callback functions.
- 2e4127b6d2d8 18.0 landed
-
Another try to fix BF failure introduced in commit ddd5f4f54a.
- 9bc1eee988c3 17.0 cited
-
Revert "Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM"
- 06bd311bce24 17.0 landed
-
Improve COPY TO performance when server and client encodings match
- b619852086ed 17.0 cited
-
Simplify signature of CopyAttributeOutCSV() in copyto.c
- b9d6038d7048 17.0 landed
-
Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"
- 1aa8324b81fa 17.0 landed
-
Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO
- 2889fd23be56 17.0 landed
-
Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM
- 95fb5b49024a 17.0 landed
-
Add progress reporting of skipped tuples during COPY FROM.
- 729439607ad2 17.0 cited
-
pgbench: Add \syncpipeline
- 94edfe250c6a 17.0 cited
-
meson: Make gzip and tar optional
- 9ca6e7b9411e 17.0 cited
-
Export the external file reader used in COPY FROM as APIs.
- 8ddc05fb01ee 9.1.0 cited