On Mon, Mar 04, 2024 at 02:11:08PM +0900, Sutou Kouhei wrote: > If this is a blocker of making COPY format extendable, can > we defer moving the existing text/csv/binary format > implementations to Copy{From,To}Routine for now as Michael > suggested to proceed making COPY format extendable? (Can we > add Copy{From,To}Routine without changing the existing > text/csv/binary format implementations?)
Yeah, I assume that it would be the way to go so as we don't do any dispatching in default cases. A different approach that could be done is to hide some of the parts of binary and text/csv in inline static functions that are equivalent to the routine callbacks. That's similar to the previous versions of the patch set, but if we come back to the argument that there is a risk of blocking optimizations of more of the local areas of the per-row processing in NextCopyFrom() and CopyOneRowTo(), what you have sounds like a good balance. CopyOneRowTo() could do something like that to avoid the extra indentation: if (cstate->routine) { cstate->routine->CopyToOneRow(cstate, slot); MemoryContextSwitchTo(oldcontext); return; } NextCopyFrom() does not need to be concerned by that. > I attach a patch for it. > There is a large hunk for CopyOneRowTo() that is caused by > indent change. I also attach "...-w.patch" that uses "git > -w" to remove space only changes. "...-w.patch" is only for > review. We should use .patch without -w for push. I didn't know this trick. That's indeed nice.. I may use that for other stuff to make patches more presentable to the eyes. And that's available as well with `git diff`. If we basically agree about this part, how would the rest work out with this set of APIs and the possibility to plug in a custom value for FORMAT to do a pg_proc lookup, including an example of how these APIs can be used? -- Michael
signature.asc
Description: PGP signature