Hi Surafel,
Andrew and I began reviewing your patch. It applied cleanly and seems to mostly have the functionality you describe. We
didhave some comments/questions.
1. It sounded like you added the copy_max_error_limit GUC as part of this patch to allow users to specify how many
errorsthey want to swallow with this new functionality. The GUC didn't seem to be defined and we saw no mention of it
inthe code. My guess is this might be good to address one of the concerns mentioned in the initial email thread of this
generatingtoo many transaction IDs so it would probably be good to have it.
2. I was curious why you only have support for skipping errors on UNIQUE and EXCLUSION constraints and not other kinds
ofconstraints? I'm not sure how difficult it would be to add support for those, but it seems they could also be
useful.
3. We think the wording "ON CONFLICT IGNORE" may not be the clearest description of what this is doing since it is
writingthe failed rows to a file for a user to process later, but they are not being ignored. We considered things like
STASHor LOG as alternatives to IGNORE. Andrew may have some other suggestions for wording.
4. We also noticed this has no tests and thought it would be good to add some to ensure this functionality works how
youintend it and continues to work. We started running some SQL to validate this, but haven't gotten the chance to put
itinto 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
intotests ourselves that we could contribute to this patch.
Thanks for writing this patch!
Karen
