Re: Make COPY format extendable: Extract COPY TO format implementations
Sutou Kouhei <kou@clear-code.com>
Hi,
In <CAD21AoBuEqcz2_+dpA3WTiDUF=FgudPBKwM+nvH+qHT-k4p5mA@mail.gmail.com>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Thu, 1 May 2025 12:15:30 -0700,
Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> One of the primary considerations we need to address is the treatment
> of the specified format name. The current patch set utilizes built-in
> formats (namely 'csv', 'text', and 'binary') when the format name is
> either unqualified or explicitly specified with 'pg_catalog' as the
> schema. In all other cases, we search for custom format handler
> functions based on the search_path. To be frank, I have reservations
> about this interface design, as the dependence of the specified custom
> format name on the search_path could potentially confuse users.
How about requiring schema for all custom formats?
Valid:
COPY ... TO ... (FORMAT 'text');
COPY ... TO ... (FORMAT 'my_schema.jsonlines');
Invalid:
COPY ... TO ... (FORMAT 'jsonlines'); -- no schema
COPY ... TO ... (FORMAT 'pg_catalog.text'); -- needless schema
If we require "schema" for all custom formats, we don't need
to depend on search_path.
> In light of these concerns, I've been contemplating alternative
> interface designs. One promising approach would involve registering
> custom copy formats via a C function during module loading
> (specifically, in _PG_init()). This method would require extension
> authors to invoke a registration function, say
> RegisterCustomCopyFormat(), in _PG_init() as follows:
>
> JsonLinesFormatId = RegisterCustomCopyFormat("jsonlines",
> &JsonLinesCopyToRoutine,
> &JsonLinesCopyFromRoutine);
>
> The registration function would validate the format name and store it
> in TopMemoryContext. It would then return a unique identifier that can
> be used subsequently to reference the custom copy format extension.
I don't object the suggested interface because I don't have
a strong opinion how to implement this feature.
Why do we need to assign a unique ID? For performance? For
RegisterCustomCopyFormatOption()?
I think that we don't need to use it so much in COPY. We
don't need to use format name and assigned ID after we
retrieve a corresponding Copy{To,From}Routine. Because all
needed information are in Copy{To,From}Routine.
> Extensions could register their own options within this
> framework, for example:
>
> RegisterCustomCopyFormatOption(JsonLinesFormatId,
> "custom_option",
> custom_option_handler);
Can we defer to discuss how to add support for custom
options while we focus on the first implementation? Earlier
patch sets with the current approach had custom options
support but it's removed in the first implementation.
(BTW, I think that it's not a good API because we want COPY
FROM only options and COPY TO only options something like
"compression level".)
> This approach offers several advantages: it would eliminate the
> search_path issue, provide greater flexibility, and potentially
> simplify the overall interface for users and developers alike.
What contributes to the "flexibility"? Developers can call
multiple Register* functions in _PG_Init(), right?
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