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

Junwang Zhao <zhjwpku(at)gmail(dot)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