On Wed, Feb 28, 2024 at 12:10:00PM +0530, Bharath Rupireddy wrote:
> On Mon, Feb 26, 2024 at 5:47 PM torikoshia <torikos...@oss.nttdata.com> wrote:
>> + 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)"?
>
> Nice catch. When COPY_ON_ERROR_STOP is specified, we use ereport's
> soft error mechanism. An assertion seems a good choice to validate the
> state is what we expect. Done that way.Hmm. I am not really on board with this patch, that would generate one NOTICE message each time a row is incompatible in the soft error mode. If you have a couple of billion rows to bulk-load into the backend and even 0.01% of them are corrupted, you could finish with a more than 100k log entries, and all systems should be careful about the log quantity generated, especially if we use the syslogger which could become easily a bottleneck. The existing ON_ERROR controls what to do on error. I think that we'd better control the amount of information reported with a completely separate option, an option even different than where to redirect errors (if required, which would be either the logs, the client, a pipe, a combination of these or even all of them). -- Michael
signature.asc
Description: PGP signature
