Thanks for looking at it
1. It sounded like you added the copy_max_error_limit GUC as part of this patch
to allow users to specify how many errors they want to swallow with this new
functionality. The GUC didn't seem to be defined and we saw no mention of it in
the code. My guess is this might be good to address one of the concerns
mentioned in the initial email thread of this generating too many transaction
IDs so it would probably be good to have it.
By mistake I write it copy_max_error_limit here but in the patch it is
copy_maximum_error_limit with a default value of 100 sorry for mismatch
2. I was curious why you only have support for skipping errors on UNIQUE and
EXCLUSION constraints and not other kinds of constraints? I'm not sure how
difficult it would be to add support for those, but it seems they could also be
useful.
I see it now that most of formatting error can be handle safely I will attache
the patch for it this week
3. We think the wording "ON CONFLICT IGNORE" may not be the clearest
description of what this is doing since it is writing the failed rows to a file
for a user to process later, but they are not being ignored. We considered
things like STASH or LOG as alternatives to IGNORE. Andrew may have some other
suggestions for wording.
I agree.I will change it to ON CONFLICT LOG if we can’t find better naming
4. We also noticed this has no tests and thought it would be good to add some
to ensure this functionality works how you intend it and continues to work. We
started running some SQL to validate this, but haven't gotten the chance to put
it into a clean test yet. We can send you what we have so far, or we are also
willing to put a little time in to turn it into tests ourselves that we could
contribute to this patch.
okay
