Adding Support for Copy callback functionality on COPY TO api

Previous Topic Next Topic
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view

Adding Support for Copy callback functionality on COPY TO api

Sanaba, Bilva

Hi hackers,


Currently, the COPY TO api does not support callback functions, while the COPY FROM api does. The COPY TO code does, however, include placeholders for supporting callbacks in the future. 


Rounding out the support of callback functions to both could be very beneficial for extension development. In particular, supporting callbacks for COPY TO will allow developers to utilize the preexisting command in order to create tools that give users more support for moving data for storage, backup, analytics, etc. 


We are aiming to get the support in core PostgreSQL and add COPY TO callback support in the next commitfest.  The attached patch contains a change to COPY TO api to support callbacks. 





0001-Support-COPY-TO-callback-functions.patch (6K) Download Attachment
Reply | Threaded
Open this post in threaded view

Re: Adding Support for Copy callback functionality on COPY TO api

Soumyadeep Chakraborty
Hi Bilva,

Thank you for registering this patch!

I had a few suggestions:

1. Please run pg_indent[1] on your code. Make sure you add
copy_data_destination_cb to src/tools/pgindent/typedefs.list. Please
run pg_indent on only the files you changed (it will take files as
command line args)

2. For features such as this, it is often helpful to find a use case
within backend/utility/extension code that demonstrate thes callback and
to include the code to exercise it with the patch. Refer how
copy_read_data() is used as copy_data_source_cb, to copy the data from
the query results from the WAL receiver (Refer: copy_table()). Finding
a similar use case in the source tree will make a stronger case
for this patch.

3. Wouldn't we want to return the number of bytes written from
copy_data_destination_cb? (Similar to copy_data_source_cb) We should
also think about how to represent failure. Looking at CopySendEndOfRow(),
we should error out like we do for the other copy_dests after checking the
return value for the callback invocation.

> bool pipe = (cstate->filename == NULL) && (cstate->data_destination_cb == NULL);

I think a similar change should also be applied to BeginCopyFrom() and
CopyFrom(). Or even better, such code could be refactored to have a
separate destination type COPY_PIPE. This of course, will be a separate
patch. I think the above line is okay for this patch.