On 2024/10/21 18:30, Kirill Reshke wrote:
v4 no longer applies. It now conflicts with
e7834a1a251d4a28245377f383ff20a657ba8262.
Also, there were review comments.

So, I decided to rebase.

Thanks for the patch! Here are my review comments:

I noticed that on_error=set_to_null does not trigger NOTICE messages for rows
and columns with errors. It's "unexpected" thing for columns to be silently
replaced with NULL due to on_error=set_to_null. So, similar to on_error=ignore,
there should be NOTICE messages indicating which input records had columns
set to NULL because of data type incompatibility. Without these messages,
users might not realize that some columns were set to NULL.


How should on_error=set_to_null behave when reject_limit is set? It seems
intuitive to trigger an error if the number of rows with columns' data type
issues exceeds reject_limit, similar to on_error=ignore. This is open to 
discussion.


psql's tab-completion should be updated to include SET_TO_NULL.


       An <replaceable class="parameter">error_action</replaceable> value of
       <literal>stop</literal> means fail the command, while
       <literal>ignore</literal> means discard the input row and continue with 
the next one.
+      <literal>set_to_null</literal> means the input value will be set to 
<literal>null</literal> and continue with the next one.

How about merging these two descriptions to one and updating it to the 
following?

-------------------
An error_action value of stop means fail the command, ignore means discard the 
input
row and continue with the next one, and set_to_null means replace columns with 
invalid
input values with NULL and move to the next row.
-------------------


      The <literal>ignore</literal> option is applicable only for <command>COPY 
FROM</command>

This should be "... ignore and set_to_null options are ..."?

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION



Reply via email to