Re: Change COPY ... ON_ERROR ignore to ON_ERROR ignore_row

On Tue, 18 Mar 2025 at 09:26, jian he <jian.universality@gmail.com> wrote:
>
> changed based on this.
>
> also minor documentation tweaks.

Few comments:
1) I felt this is wrong:
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 9a4d993e2bc..7980513a9bd 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -3280,7 +3280,7 @@ match_previous_words(int pattern_id,
                COMPLETE_WITH("FORMAT", "FREEZE", "DELIMITER", "NULL",
                                          "HEADER", "QUOTE", "ESCAPE",
"FORCE_QUOTE",
                                          "FORCE_NOT_NULL",
"FORCE_NULL", "ENCODING", "DEFAULT",
-                                         "ON_ERROR", "LOG_VERBOSITY");
+                                         "ON_ERROR", "SET_TO_NULL",
"LOG_VERBOSITY");

as the following fails:
postgres=# copy t_on_error_null from stdin WITH ( set_to_null );
ERROR:  option "set_to_null" not recognized
LINE 1: copy t_on_error_null from stdin WITH ( set_to_null );

2) Can you limit this to 80 chars if possible to improve the readability:
+      <literal>stop</literal> means fail the command,
+      <literal>ignore</literal> means discard the input row and
continue with the next one, and
+      <literal>set_to_null</literal> means replace columns containing
invalid input values with
+      <literal>NULL</literal> and move to the next field.

3) similarly here too:
+      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_to_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.

4) Could you mention a brief one line in the commit message as to why
"on_error null" cannot be used:
Extent "on_error action", introduce new option:  on_error set_to_null.
Current grammar makes us unable to use "on_error null", so we choose
"on_error set_to_null".

Regards,
Vignesh