Re: [HACKERS] GSOC'17 project introduction: Parallel COPY execution with errors handling

Alexey Kondratov wrote:

> I have rebased my branch and squashed all commits into one, since the
> patch is quite small. Everything seems to be working fine now, patch
> passes all regression tests.

On a *very* quick look, please use an enum to return from NextCopyFrom
rather than 'int'. The chunks that change bool to int are very
odd-looking. This would move the comment that explains the value from
copy.c to copy.h, obviously. Also, you seem to be using non-ASCII dashes
in the descriptions of those values; please don't.

But those are really just minor nitpicks while paging through. After
looking at the way you're handling errors, it seems that you've chosen
to go precisely the way people were saying that was not admissible: use
a PG_TRY block, and when things go south, simply log a WARNING and move
on. This is unsafe. For example: what happens if the error being
raised is out-of-memory? Failing to re-throw means you don't release
current transaction memory context. What if the error is that WAL
cannot be written because the WAL disk ran out of space? This would
just be reported as a warning over and over until the server log disk is
also full. What if your insertion fired a trigger that caused an update
which got a serializability exception? You cannot just move on, because
at that point session state (serializability session state) might be
busted until you abort the current transaction.

Or maybe I misunderstood the patch completely.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services