Re: Add new error_action COPY ON_ERROR "log"

On 2024-02-20 17:22, torikoshia wrote:
> On 2024-02-17 15:00, Bharath Rupireddy wrote:
>> On Fri, Feb 16, 2024 at 8:17 PM torikoshia
>> <torikoshia(at)oss(dot)nttdata(dot)com> wrote:
>>>
>>> I may be wrong since I seldom do data loading tasks, but I greed with
>>> you.
>>>
>>> I also a little concerned about the case where there are many
>>> malformed
>>> data and it causes lots of messages, but the information is usually
>>> valuable and if users don't need it, they can suppress it by changing
>>> client_min_messages.
>>>
>>> Currently both summary of failures and individual information is
>>> logged
>>> in NOTICE level.
>>> If we should assume that there are cases where only summary
>>> information
>>> is required, it'd be useful to set lower log level, i.e. LOG to the
>>> individual information.
>>
>> How about we emit the summary at INFO level and individual information
>> at NOTICE level? With this, the summary is given a different priority
>> than the individual info. With SET client_min_messages = WARNING; one
>> can still get the summary but not the individual info. Also, to get
>> all of these into server log, one can SET log_min_messages = INFO; or
>> SET log_min_messages = NOTICE;.
>>
>> Thoughts?
>
> It looks good to me.

Here are comments on the v2 patch.

+ if (cstate->opts.on_error != COPY_ON_ERROR_STOP)
+ {
+ ereport(NOTICE,

I think cstate->opts.on_error is not COPY_ON_ERROR_STOP here, since if
it is COPY_ON_ERROR_STOP, InputFunctionCallSafe() should already have
errored out.

Should it be something like "Assert(cstate->opts.on_error !=
COPY_ON_ERROR_STOP)"?

Should below manual also be updated?

> A NOTICE message containing the ignored row count is emitted at the end
> of the COPY FROM if at least one row was discarded.

--
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation