On Mon, Feb 26, 2024 at 5:47 PM torikoshia <torikos...@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
v3-0001-Add-detailed-info-when-COPY-skips-soft-errors.patch
Description: Binary data
