From: | Soumyadeep Chakraborty <soumyadeep2007(at)gmail(dot)com> |
---|---|
To: | "Sanaba, Bilva" <bilvas(at)amazon(dot)com> |
Cc: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Adding Support for Copy callback functionality on COPY TO api |
Date: | 2020-09-14 23:28:12 |
Message-ID: | CAE-ML+_ebFzUnsgj-4MLshLGFfT05Lo15KK6rpcUt8-ZZKL1XQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
4.
> 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.
Regards,
Soumyadeep
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2020-09-15 00:08:58 | Re: pg_restore causing deadlocks on partitioned tables |
Previous Message | David Rowley | 2020-09-14 23:17:24 | Re: Use incremental sort paths for window functions |