Horton

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

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Junwang Zhao <zhjwpku@gmail.com>
Cc: Sutou Kouhei <kou@clear-code.com>, sawada.mshk@gmail.com, michael@paquier.xyz, pgsql-hackers@postgresql.org
Date: 2025-03-02T16:27:20Z
Lists: pgsql-hackers 
Junwang Zhao <zhjwpku@gmail.com> writes:
> While review another thread (Emitting JSON to file using COPY TO),
> I found the recently committed patches on this thread pass the
> CopyFormatOptions struct directly rather a pointer of the struct
> as a function parameter of CopyToGetRoutine and CopyFromGetRoutine.

Coverity is unhappy about that too:

/srv/coverity/git/pgsql-git/postgresql/src/backend/commands/copyto.c: 177 in CopyToGetRoutine()
171     	.CopyToOneRow = CopyToBinaryOneRow,
172     	.CopyToEnd = CopyToBinaryEnd,
173     };
174     
175     /* Return a COPY TO routine for the given options */
176     static const CopyToRoutine *
>>>     CID 1643911:  Performance inefficiencies  (PASS_BY_VALUE)
>>>     Passing parameter opts of type "CopyFormatOptions" (size 184 bytes) by value, which exceeds the low threshold of 128 bytes.
177     CopyToGetRoutine(CopyFormatOptions opts)
178     {
179     	if (opts.csv_mode)
180     		return &CopyToRoutineCSV;

(and likewise for CopyFromGetRoutine).  I realize that these
functions aren't called often enough for performance to be an
overriding concern, but it still seems like poor style.

> Then I took a quick look at the newly rebased patch set and
> found Sutou has already fixed this issue.

+1, except I'd suggest declaring the parameters as
"const CopyFormatOptions *opts".

			regards, tom lane

Commits

  1. Refactor Copy{From|To}GetRoutine() to use pass-by-reference argument.

  2. Refactor COPY FROM to use format callback functions.

  3. Refactor COPY TO to use format callback functions.

  4. Another try to fix BF failure introduced in commit ddd5f4f54a.

  5. Revert "Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM"

  6. Improve COPY TO performance when server and client encodings match

  7. Simplify signature of CopyAttributeOutCSV() in copyto.c

  8. Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"

  9. Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO

  10. Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM

  11. Add progress reporting of skipped tuples during COPY FROM.

  12. pgbench: Add \syncpipeline

  13. meson: Make gzip and tar optional

  14. Export the external file reader used in COPY FROM as APIs.