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

Hi,

Thank you all for keeping this going. I looked at this patch set from the
user side: people often ask for COPY to and from Parquet, and as I
understand the thread, this API is how such a format would live in an
extension instead of in core.

I reviewed v3 (Sawada-san, 2026-06-24), together with Manu's export diff and
ndjson module (2026-09-22).

Tested against master c074737f687f604b64f6ee830fccbbe854719823 (2026-11-09),
Debian bookworm container, ./configure --enable-cassert --enable-debug
--enable-tap-tests.

Applying and building
- v3 no longer applies with git apply on current master. git am -3 works for
0001 to 0003 (copyto.c, copyfrom.c, copyfromparse.c and file_fdw.c merge
automatically, no conflicts). In 0004 only the typedefs.list hunk fails;
the rest applies.
- Build is clean, with no compiler warnings.

Tests
- make check: 243 tests, PASS. contrib/file_fdw check: PASS.
- test_copy_custom_format fails on current master, as Manu reported. I think
the cause is in psql, not in the patch. Commit 3045a25ba8 (2026-08-10,
"Teach psql to skip in-line COPY ... FROM STDIN data after a failure")
makes psql skip input up to "\." after a COPY FROM STDIN that fails. The
test has a COPY FROM STDIN that is expected to fail (the disallow_freeze
case) with no "\." after it, so psql reads the next COPY command as data,
and its output is missing. The server raises the error in
ProcessCopyOptions(), before CopyInResponse, so the backend side is fine.
The same happens with a built-in format, for example COPY t FROM stdin
WITH (format 'text', bogus 1). Adding one line "\." after the failing
command makes the test pass, with the expected file unchanged.
- Manu's ndjson module: without the export diff it compiles with implicit
declaration warnings, and LOAD fails with "undefined symbol:
CopySendData". With the export diff, his round-trip script passes (file,
column lists), and so do \copy through the frontend protocol and COPY TO
PROGRAM.

Comments

1. Exporting CopySendData(), CopySendEndOfRow() and CopyGetData(): +1.
Without them a format can be registered but not implemented, so I think they
belong in 0002. As Manu said, the comment should mention that
CopySendEndOfRow() does not add a newline.

2. Core options that a custom format does not use are accepted silently.
With FORMAT ndjson, all of these run without an error and do not change the
output: DELIMITER '|', HEADER true, NULL 'x', FORCE_QUOTE *. Since the
text/CSV checks are now skipped for custom formats (CopyFormatIsBuiltins()),
each format has to reject these in its validate callback, and if it does
not, the user's option is ignored without notice. Maybe core could reject
the text/CSV-only options for a custom format unless the format says it
accepts them. This may be related to the open question about an
InitializeCopyOptions callback.

3. ON_ERROR and line numbers. COPY ... FROM ... WITH (FORMAT ndjson,
ON_ERROR ignore) is accepted, but a bad row still fails with "invalid input
syntax for type integer", and the context line says "COPY ndj2, line 0".
This is up to the module (it would need InputFunctionCallSafe() and to
update cur_lineno), but it shows that the contract around on_error,
reject_limit, log_verbosity and cur_lineno is not written down. A short
paragraph in copyapi.h, or a way for a format to say it does not support
ON_ERROR so that core rejects it, would help extension authors.

4. The library has to be loaded first. In a new session without LOAD, FORMAT
ndjson fails with 'COPY format "ndjson" not recognized'. This is expected
with registration in _PG_init(), so users will need shared_preload_libraries
or session_preload_libraries. An errhint pointing to that could save some
confusion. Small point.

5. Parquet as a use case. I tried to picture a Parquet format on top of this
API:
- COPY TO seems to fit: buffer rows in CopyToOneRow, write the row groups
and the footer in CopyToEnd through CopySendData(), and keep the state
in format_private.
- COPY FROM is harder, because the Parquet footer is at the end of the
file, so the reader needs random access. For a file source the
extension could use cstate->copy_file, which copy_state.h now exposes.
For STDIN or PROGRAM it would have to read everything through
CopyGetData() first. That is fine, but it would help to document
whether a format may use copy_file directly or should only use
CopyGetData().
- A target like 's3://bucket/x.parquet' is rejected by core ("relative
path not allowed for COPY to file") before the format sees it. So an
extension that writes to object storage would still need its own hook
for that part. I think that is fine and out of scope here; the API
covers the format, not the destination.

6. file_fdw works with a custom format with no extra code: a foreign table
with format 'ndjson' reads the file correctly. A nice side effect. If it is
intended, a test or a line in the docs would make it explicit.

7. Small things:
- Typo "Sturct" in copyapi.h, and "Add an hook" in the 0003 subject ("a
hook").
- The commit messages of 0002 and 0003 have empty Author, Reviewed-by and
Discussion lines.
- There are no SGML docs yet; copy.sgml still lists only the built-in
formats.
- I only built with autoconf, not meson.

Verdict: the registration API works as described, and with the export diff I
could run a real format on top of it. The patch set needs a rebase. I would
suggest folding the export diff into 0002 and adding the "\." line to the
test. Comments 2 and 3 are the ones I think matter most for format authors;
the rest are small.

Thanks again.

Regards,
Joao Rito