I have a few more cosmetic changes to suggest:
- doc/src/sgml/ref/copy.sgml
+ and <literal>set_null</literal> means replace field containing
invalid
should be "the field" and "the invalid"
+ input value with <literal>NULL</literal> and continue to the next
field.
change <literal>NULL</literal> to "a null value"
+ <para>
+ For <literal>ignore</literal> option, a <literal>NOTICE</literal>
message
+ containing the ignored row count is emitted at the end of the
<command>COPY FROM</command>
+ if at least one row was discarded.
+ For <literal>set_null</literal> option, a <literal>NOTICE</literal>
+ message indicating the number of rows where invalid input values were
+ replaced with null is emitted at the end of the <command>COPY
FROM</command>
+ if at least one row was replaced.
+ </para>
I think this could be written more compactly, like
If on_error is set to ignore or set_null, a NOTICE message is emitted
at the end of the COPY FROM command containing the count of rows that
were ignored or changed, if at least one row was affected.
- src/backend/commands/copy.c
/*
- * Allow "stop", or "ignore" values.
+ * Allow "stop", "ignore", "set_null" values.
*/
Just remove that comment. It is evident from the following code.
- src/backend/commands/copyfrom.c
+ "%" PRIu64 " rows were replaced with null due to data type
incompatibility"
I think this is not quite correctly worded. It should be something like
in NNN rows, columns were set to null due to ...
because you are not setting the whole row to null.
/*
- * Currently we only support COPY_ON_ERROR_IGNORE. We'll add other
- * options later
+ * Currently we only support COPY_ON_ERROR_IGNORE,
+ * COPY_ON_ERROR_SET_NULL. We'll add other options later
*/
Delete this comment.
+ if (cstate->opts.on_error == COPY_ON_ERROR_SET_NULL)
+ {
+ int attr_count = list_length(cstate->attnumlist);
+
+ cstate->domain_with_constraint = palloc0_array(bool, attr_count);
Maybe add a comment for this block to explain that you are collecting
information about domains for later.
- src/backend/commands/copyfromparse.c
/*
- * If ON_ERROR is specified with IGNORE, skip rows with soft errors
+ * If ON_ERROR is specified with IGNORE, skip rows with soft errors.
+ * If ON_ERROR is specified with SET_NULL, try to replace with null.
*/
Trim this comment. Maybe "If ON_ERROR is specified, handle the
different options". We don't need to re-explain here what the options do.
+ /*
+ * If the column type is a constrained domain, an additional
+ * InputFunctionCallSafe may be needed to raise error for
+ * domain constraint violation.
+ */
Why "may be needed"? Is it sometimes not needed? Why, under what
circumstances?
The subsequent error message writes "domain ... does not allow null
values", but AFAICT a domain input failure could also be due to a check
constraint failure? How would that be handled? The flow here is a bit
confusing.
- src/test/regress/sql/copy2.sql
I suggest adding a space after "--" in a comment, like "-- error"
instead of "--error".
Similarly, a space after CHECK, like "CHECK (...)".
