Re: Make COPY format extendable: Extract COPY TO format implementations
Sutou Kouhei <kou@clear-code.com>
Hi, In <CAD21AoBGRFStdVbHUcxL0QB8wn92J3Sn-6x=RhsSMuhepRH0NQ@mail.gmail.com> "Re: Make COPY format extendable: Extract COPY TO format implementations" on Fri, 2 May 2025 21:38:32 -0700, Masahiko Sawada <sawada.mshk@gmail.com> wrote: >> 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. > > I'm concerned that users cannot use the same format name in the FORMAT > option depending on which schema the handler function is created. I'm not sure that it's a problem or not. If users want to use the same format name, they can install the handler function to the same schema. >> Why do we need to assign a unique ID? For performance? For >> RegisterCustomCopyFormatOption()? > > I think it's required for monitoring purposes for example. For > instance, we can set the format ID in the progress information and the > progress view can fetch the format name by the ID so that users can > see what format is being used in the COPY command. How about setting the format name instead of the format ID in the progress information? > I think we can skip the custom option patch for the first > implementation but still need to discuss how we will be able to > implement it to understand the big picture of this feature. Otherwise > we could end up going the wrong direction. I think that we don't need to discuss it deeply because we have many options with this approach. We can call C functions in _PG_Init(). I think that this feature will not be a blocker of this approach. >> (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".) > > Why does this matter in terms of API? I think that even with this API > we can pass is_from to the option handler function so that it > validates the option based on it. If we choose the API, each custom format developer needs to handle the case in handler function. For example, if we pass information whether this option is only for TO to PostgreSQL, ProcessCopyOptions() not handler functions can handle it. Anyway, I think that we don't need to discuss this deeply for now. >> What contributes to the "flexibility"? Developers can call >> multiple Register* functions in _PG_Init(), right? > > I think that with a tablesample-like approach we need to do everything > based on one handler function and callbacks returned from it whereas > there is no such limitation with C API style. Thanks for clarifying it. It seems that my understanding is correct. I hope that the flexibility is needed flexibility and too much flexibility doesn't introduce too much complexity. 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