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

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
of128 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