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

Hello Masahiko, Sutou-san, everyone,

You closed your last message with:

> I'll verify that the new API works well with an experimental custom
> copy format extension.

I went ahead and did that, since the entry has been sitting since June.
I wrote a real custom format against v3 - newline-delimited JSON, both
directions, attached - and the short answer is that the API shape works,
but an extension cannot currently implement a format with it.

Everything below is on master 09a579abaca, v3 applied (0004 needs a
one-line rebase: typedefs.list now has TestCustomScanState where the
hunk expects to insert).

1. An extension cannot move any bytes
--------------------------------------

The callbacks are fine, but the functions that talk to the COPY source
and destination are all static:

copyto.c: CopySendData, CopySendEndOfRow,
CopySendTextLikeEndOfRow
copyfromparse.c: CopyGetData, CopyReadLine

copy_state.h exports no functions at all. So a format can be registered
and its callbacks called, but CopyToOneRow has no way to hand its bytes
to the destination, and CopyFromOneRow has no way to pull bytes from
the source. The destination can be a file, a PROGRAM, the frontend or a
callback, and only CopySendEndOfRow/CopyGetData know the difference;
reimplementing that in every extension is not an option.

I think this is why test_copy_custom_format only emits NOTICEs: it is
the most an extension can do today, and that is also why the test
module cannot catch this.

Attached nocfbot-export-copy-io.diff.txt exports the three that a format
actually needs (CopySendData, CopySendEndOfRow, CopyGetData) and
declares them in copyapi.h. With that diff on top of v3, the ndjson
extension compiles and works:

COPY ndj TO STDOUT WITH (FORMAT ndjson)
{"id":"1","name":"hello","amount":"1.25","ok":"t","ts":"2026-01-31"}
{"id":"2","name":"with \"quotes\" and \\ backslash",...}
{"id":"3","name":null,"amount":null,"ok":null,"ts":null}

and a full round trip through a file returns the data unchanged:
quotes, backslashes, embedded tabs and newlines, NULLs and a leap-year
date all survive, and both EXCEPT ALL checks come back empty. A column
list is honoured on both sides, and keys missing from a line come back
as NULL.

Two smaller things I hit while writing it, both worth a line of
documentation in copyapi.h rather than a code change:

* CopySendEndOfRow does not terminate the row. The built-in text/CSV
formats go through CopySendTextLikeEndOfRow for that, which stays
static, so a line-oriented custom format has to append its own "\n".
Nothing says so, and there is no example to copy from.
* The format gets its state through cstate->format_private, which is
never mentioned in the API comments.

2. The performance question
----------------------------

This thread has been circling performance since 2023, and in December
Sutou-san's six runs showed no reproducible trend while you noted that
0001-from-binary and 0006-to-binary looked slower in all six.

I think the reason those results never settle is that the effect being
chased is smaller than the measurement noise. What v3 changes on the
built-in path is in ProcessCopyOptions(), which runs once per COPY
command, not per row - so a large COPY cannot see it by construction.

I measured anyway, 1M rows, {1,10,100} int columns, text/csv/binary,
COPY TO and FROM, 7 repetitions, pinned to one core (scripts attached).
The first pass suggested text COPY FROM with 100 columns was 11% slower
under v3. That turned out to be an artifact of my own harness: master
always ran first within a repetition. Re-running the suspicious cases
in both orders, 10 repetitions each:

text FROM 100 cols, master first: +4.9%
text FROM 100 cols, v3 first: +2.7%
binary FROM 100 cols, master first: -2.2%
binary FROM 100 cols, v3 first: +1.3%

binary changes sign with the order, so that one was the harness. text
keeps its sign, so something is there. I then built a tree with only
0001 applied - the patch that just moves the structs into copy_state.h
and renames the COPY_FILE/COPY_FRONTEND enums, with no execution-path
change at all - and measured it the same way:

text FROM 100 cols, master first: +2.9% v-first: +0.5%
binary FROM 100 cols, master first: +4.3% v-first: +2.6%

A patch that only moves declarations between headers cannot cost 3-4%
of a COPY. So what these numbers are showing is code layout and
measurement noise, not the cost of the API - which also explains the
contradictory heatmaps from earlier in the thread. I would suggest not
letting this block the design any further; if someone wants a real
answer it needs a harness built for it, not bigger COPYs.

I did not benchmark a custom format against a built-in one, since that
compares two different amounts of work.

3. v3 does not pass its own test
---------------------------------

make -C src/test/modules/test_copy_custom_format check -> FAIL

and it fails on a plain v3 tree, with none of my changes applied, so
this is not something I introduced. The diff is this part of the
expected output never happening:

COPY copy_data FROM stdin WITH (format 'test_format',
disallow_freeze true); -- OK
NOTICE: CopyFromInFunc: attribute: smallint
...

The cause is the line just before it in the test script:

COPY copy_data FROM stdin WITH (format 'test_format', freeze true,
disallow_freeze true); -- ERROR

Reproduced by hand: the server has already switched the connection to
copy-in mode by the time the validation callback raises, so psql
swallows the *next* line - the second COPY - as data for the failed
one, together with the terminating \. The second statement never runs.

Two statements, both COPY ... FROM stdin, one of which is meant to
fail: the error case needs to go last, or the OK case needs its own
input. Worth deciding whether the validation callback should run
before the connection flips to copy-in, which would also make this
test behave the way it reads.

4. Also
--------

make check, src/test/modules (other than the above) and
src/bin/pg_dump pass with v3.

Sutou-san asked for CopyFormatIsBuiltin() back in June; v3 has
CopyFormatIsBuiltins(), still plural for a macro that returns a
boolean.

Happy to turn the ndjson module into a proper test module for the
series if you think it is worth having something in the tree that
actually moves data through the API. It is attached as plain .txt for
now.

Regards,
Manu