Re: Add new error_action COPY ON_ERROR "log"

On Mon, Feb 26, 2024 at 5:47 PM torikoshia <torikoshia@oss.nttdata.com> wrote:
>
> >
> > It looks good to me.
>
> Here are comments on the v2 patch.

Thanks for looking at it.

> +               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.

> 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.

Changed.

PSA v3 patch with the above review comments addressed.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com