Re: Change COPY ... ON_ERROR ignore to ON_ERROR ignore_row

On Sat, Nov 9, 2024 at 8:55 PM Kirill Reshke <reshkekirill@gmail.com> wrote:
>

> > > But while I was trying to implement that, I realized that I don't
> > > understand v4 of this patch. My misunderstanding is about
> > > `t_on_error_null` tests. We are allowed to insert a NULL value for the
> > > first column of t_on_error_null using COPY ON_ERROR SET_TO_NULL. Why
> > > do we do that? My thought is we should try to execute
> > > InputFunctionCallSafe with NULL value  (i mean, here [1]) for the
> > > column after we failed to insert the input value. And, if this second
> > > call is successful, we do replacement, otherwise we count the row as
> > > erroneous.
> >
> > Your concern is valid. Allowing NULL to be stored in a column with a NOT NULL
> > constraint via COPY ON_ERROR=SET_TO_NULL does seem unexpected. As you suggested,
> > NULL values set by SET_TO_NULL should probably be re-evaluated.
>
> Thank you. I updated the patch with a NULL re-evaluation.
>


take me sometime to understand your change with InputFunctionCallSafe.
it actually works fine with domain,
i think mainly because domain_in proisstrict is false and all other
type input function proisstrict is true!


--case1
create table t1(a dnn);
copy t1 from stdin(on_error set_to_null);
A
\.

--case2
create table t2(a int not null);
copy t2 from stdin(on_error set_to_null);
A
\.

I think it should be to either let domains with not-null behave the
same as column level not-null
or just insert NULL to a column with domain not-null constraint.


in doc[1], we already mentioned that a column with a not-null domain
is possible to have null value .
https://www.postgresql.org/docs/current/sql-createdomain.html

attached v8, based on your v7, main change it NextCopyFrom,
InputFunctionCallSafe.
The idea is when InputFunctionCallSafe fails, on_error set_to_null
needs to check if this is a type as not-null domain.
pass NULL string to InputFunctionCallSafe again to check if this type
allows null or not.
If not allow null then error out (ereport(ERROR)).
i think this will align with column level not-null constraint, what do
you guys think?


i am mainly change copyfromparse.c's for now.
other places no change, same as v7.