Horton

Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features)

Daniel Gustafsson <daniel@yesql.se>

View thread
From: Daniel Gustafsson <daniel@yesql.se>
To: Damir Belyalov <dam.bel07@gmail.com>
Cc: torikoshia <torikoshia@oss.nttdata.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>, Danil Anisimow <anisimow.d@gmail.com>, HukuToc@gmail.com, a.lepikhov@postgrespro.ru, tgl@sss.pgh.pa.us
Date: 2023-03-06T14:03:38Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. doc: Fix COPY ON_ERROR option syntax synopsis.

  2. Disallow specifying ON_ERROR option without value.

  3. Rename COPY option from SAVE_ERROR_TO to ON_ERROR

  4. Fix spelling in notice

  5. Add new COPY option SAVE_ERROR_TO

> On 28 Feb 2023, at 15:28, Damir Belyalov <dam.bel07@gmail.com> wrote:

> Tested patch on all cases: CIM_SINGLE, CIM_MULTI, CIM_MULTI_CONDITION. As expected it works.
> Also added a description to copy.sgml and made a review on patch.
> 
> I added 'ignored_errors' integer parameter that should be output after the option is finished.
> All errors were added to the system logfile with full detailed context. Maybe it's better to log only error message.

FWIW, Greenplum has a similar construct (but which also logs the errors in the
db) where data type errors are skipped as long as the number of errors don't
exceed a reject limit.  If the reject limit is reached then the COPY fails:

	LOG ERRORS [ SEGMENT REJECT LIMIT <count> [ ROWS | PERCENT ]]

IIRC the gist of this was to catch then the user copies the wrong input data or
plain has a broken file.  Rather than finding out after copying n rows which
are likely to be garbage the process can be restarted.

This version of the patch has a compiler error in the error message:

copyfrom.c: In function ‘CopyFrom’:
copyfrom.c:1008:29: error: format ‘%ld’ expects argument of type ‘long int’, but argument 2 has type ‘uint64’ {aka ‘long long unsigned int’} [-Werror=format=]
1008 | ereport(WARNING, errmsg("Errors: %ld", cstate->ignored_errors));
     |                          ^~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~
     |                                              |
     |                                              uint64 {aka long long unsigned int}


On that note though, it seems to me that this error message leaves a bit to be
desired with regards to the level of detail.

--
Daniel Gustafsson