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

On Wed, Mar 26, 2025 at 8:28 PM Sutou Kouhei <kou(at)clear-code(dot)com> wrote:

>
> > ---
> > +static const CopyFromRoutine CopyFromRoutineTestCopyFormat = {
> > + .type = T_CopyFromRoutine,
> > + .CopyFromInFunc = CopyFromInFunc,
> > + .CopyFromStart = CopyFromStart,
> > + .CopyFromOneRow = CopyFromOneRow,
> > + .CopyFromEnd = CopyFromEnd,
> > +};
> >
>
>
In trying to document the current API I'm strongly disliking it. Namely,
the amount of internal code an extension needs to care about/copy-paste to
create a working handler.

Presently, pg_type defines text and binary I/O routines and the text/csv
formats use the text I/O while binary uses binary I/O - for all
attributes. The CopyFromInFunc API allows for each attribute to somehow
have its I/O format individualized. But I don't see how that is practical
or useful, and it adds burden on API users.

I suggest we remove both .CopyFromInFunc and .CopyFromStart/End and add a
property to CopyFromRoutine (.ioMode?) with values of either Copy_IO_Text
or Copy_IO_Binary and then just branch to either:

CopyFromTextLikeInFunc & CopyFromTextLikeStart/End
or
CopyFromBinaryInFunc & CopyFromStart/End

So, in effect, the only method an extension needs to write is converting
to/from the 'serialized' form to the text/binary form (text being near
unanimous).

In a similar manner, the amount of boilerplate within CopyFromOneRow seems
undesirable from an API perspective.

cstate->cur_attname = NameStr(att->attname);
cstate->cur_attval = string;

if (string != NULL)
nulls[m] = false;

if (cstate->defaults[m])
{
/* We must have switched into the per-tuple memory context */
Assert(econtext != NULL);
Assert(CurrentMemoryContext == econtext->ecxt_per_tuple_memory);

values[m] = ExecEvalExpr(defexprs[m], econtext, &nulls[m]);
}

/*
* If ON_ERROR is specified with IGNORE, skip rows with soft errors
*/
else if (!InputFunctionCallSafe(&in_functions[m],
string,
typioparams[m],
att->atttypmod,
(Node *) cstate->escontext,
&values[m]))
{
CopyFromSkipErrorRow(cstate);
return true;
}

cstate->cur_attname = NULL;
cstate->cur_attval = NULL;

It seems to me that CopyFromOneRow could simply produce a *string
collection,
one cell per attribute, and NextCopyFrom could do all of the above on a
for-loop over *string

The pg_type and pg_proc catalogs are not extensible so the API can and
should be limited to
producing the, usually text, values that are ready to be passed into the
text I/O routines and all
the work to find and use types and functions left in the template code.

I haven't looked at COPY TO but I am expecting much the same. The API
should simply receive
the content of the type I/O output routine (binary or text as it dictates)
for each output attribute, by
row, and be expected to take those values and produce a final output from
them.

David J.