Re: Change COPY ... ON_ERROR ignore to ON_ERROR ignore_row
Kirill Reshke <reshkekirill@gmail.com>
From: Kirill Reshke <reshkekirill@gmail.com>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: jian he <jian.universality@gmail.com>,
Jim Jones <jim.jones@uni-muenster.de>, "David G. Johnston" <david.g.johnston@gmail.com>,
Yugo NAGATA <nagata@sraoss.co.jp>, torikoshia <torikoshia@oss.nttdata.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-11-09T12:55:04Z
Lists: pgsql-hackers
Attachments
- v7-0001-Incrtoduce-COPY-option-to-replace-columns-contain.patch (application/octet-stream) patch v7-0001
On Thu, 7 Nov 2024 at 23:00, Fujii Masao <masao.fujii@oss.nttdata.com> wrote: > > > > On 2024/10/26 6:03, Kirill Reshke wrote: > > when the REJECT LIMIT is set to some non-zero number and the number of > > row NULL replacements exceeds the limit, is it OK to fail. Because > > there WAS errors, and we should not tolerate more than $limit errors . > > I do find this behavior to be consistent. > > +1 > > > > But what if we don't set a REJECT LIMIT, it is sane to do all > > replacements, as if REJECT LIMIT is inf. > > +1 After thinking for a while, I'm now more opposed to this approach. I think we should count rows with erroneous data as errors only if null substitution for these rows failed, not the total number of rows which were modified. Then, to respect the REJECT LIMIT option, we compare this number with the limit. This is actually simpler approach IMHO. What do You think? > > 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. PFA v7. I did not yet update the doc for this patch version, waiting for feedback about REJECT LIMIT + SET_TO_NULL behaviour. Best regards, Kirill Reshke
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
file_fdw: Add regression test for file_fdw with ON_ERROR='set_null'
- d8879d34b9dd 19 (unreleased) landed
-
Add COPY (on_error set_null) option
- 2a525cc97e19 19 (unreleased) landed
-
Add REJECT_LIMIT option to the COPY command.
- 4ac2a9beceb1 18.0 cited
-
Add log_verbosity = 'silent' support to COPY command.
- e7834a1a251d 18.0 cited
-
Add new COPY option LOG_VERBOSITY.
- f5a227895e17 17.0 cited