On Tue, Mar 11, 2025 at 6:31 PM Jim Jones <jim.jo...@uni-muenster.de> wrote: > > > I revisited this patch today. It applies and builds cleanly, and it > works as expected. > > Some tests and minor comments: >
hi. Jim Jones. thanks for testsing it again! > ==== > > 1) WARNING might be a better fit than NOTICE here. > but NOTICE, on_errror set_to_null is aligned with on_errror ignore. > > I would still leave the extra messages from "log_verbosity verbose" as > NOTICE though. What do you think? > > ==== When LOG_VERBOSITY option is set to verbose, for ignore option, a NOTICE message containing the line of the input file and the column name whose input conversion has failed is emitted for each discarded row; for set_to_null option, a NOTICE message containing the line of the input file and the column name where value was replaced with NULL for each input conversion failure. see the above desciption, on_errror set_to_null is aligned with on_errror ignore. it's just on_errror ignore is per row, on_errror set_to_null is per column/field. so NOTICE is aligned with other on_error option. > > 2) Inconsistent terminology. Invalid values in "on_error set_to_null" > mode are names as "erroneous", but as "invalid" in "on_error stop" mode. > I don't want to get into the semantics of erroneous or invalid, but > sticking to one terminology would IMHO look better. > I am open to changing it. what do you think "invalid values in %llu row was replaced with null"? > ==== > > "on_error ignore" works well with "reject_limit #" > i remember there was some confusion about on_error set_to_null with reject_limit option. I choose to not suport it. obviously, if there is consenses, we can support it later.