public inbox for [email protected]  
help / color / mirror / Atom feed
From: torikoshia <[email protected]>
To: Daniel Gustafsson <[email protected]>
To: [email protected]
Cc: PostgreSQL Hackers <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Danil Anisimow <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features)
Date: Tue, 07 Mar 2023 11:07:07 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CALH1Lgv5QeYSWYDgMBd=wvKum8egiaMR-_77VGsiHPt7E8dK9Q@mail.gmail.com>
	<CALH1LgvzpORM+EHVXRz4-Dv+_aHvm9m9Hu6eYM4PfppwKgXvFQ@mail.gmail.com>
	<[email protected]>
	<CALH1Lgsr7Asz6r6jnRta7DVEjKwyz=b3XJhyCMoYv8Yy9y4cXg@mail.gmail.com>
	<[email protected]>
	<CALH1LguQ5P0uZSYQFy4gJihvV11EkLx+gwxZ-+1DFq1Z6dJqbg@mail.gmail.com>
	<CALH1Lgug+mR9A1bBzR8hrp2oW4=jvR_N0An-i2X_j_y9C1fkiA@mail.gmail.com>
	<CALH1LguPAN=p5wHKwGcHrjE8Vj1x4o1z3Pk228YDif6LFzbeYQ@mail.gmail.com>
	<CABm2Ma6_=qf7kZPyOmiK1CUXeio7q08HNPYXrqkCwF3P_KQu7A@mail.gmail.com>
	<CALH1Lgv+L_HNN2NKo=WD8BbgLy7tEH7=OhNTJ2QWCU_CPMum0g@mail.gmail.com>
	<[email protected]>
	<CALH1LgsbWZWS3qMxdGjg4n57DUHxqyBgTzvYSzfi-sP1N-ZORQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CALH1LgvQ5eRMi3zO7RSmod4ucru=9ekASpReahR7zDcCabANZw@mail.gmail.com>
	<[email protected]>

On 2023-03-06 23:03, Daniel Gustafsson wrote:
>> On 28 Feb 2023, at 15:28, Damir Belyalov <[email protected]> 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.
Thanks for your tests and improvements!

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

> 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.
+1.
I felt just logging "Error: %ld" would make people wonder the meaning of 
the %ld. Logging something like ""Error: %ld data type errors were 
found" might be clearer.

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION






view thread (19+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features)
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox