On Fri, Dec 1, 2017 at 1:58 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> 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.
I will fix it, thank you.
>
> Or maybe I misunderstood the patch completely.
>
I hope so. Here is my thoughts how it all works, please correct me,
where I am wrong:
1) First, I have simply changed ereport level to WARNING for specific
validations (extra or missing columns, etc) if INGONE_ERRORS option is
used. All these checks are inside NextCopyFrom. Thus, this patch
performs here pretty much the same as before, excepting that it is
possible to skip bad lines, and this part should be safe as well
2) About PG_TRY/CATCH. I use it to catch the only one specific
function call inside NextCopyFrom--it is InputFunctionCall--which is
used just to parse datatype from the input string. I have no idea how
WAL write or trigger errors could get here
All of these is done before actually forming a tuple, putting it into
the heap, firing insert-related triggers, etc. I am not trying to
catch all errors during the row processing, only input data errors. So
why is it unsafe?
Best,
Alexey
