Re: Conflict handling for COPY FROM

2020-04-08 Thread David G. Johnston
On Fri, Mar 27, 2020 at 3:27 PM Tom Lane wrote: > Surafel Temesgen writes: > > [ conflict-handling-copy-from-v16.patch ] > > I took a quick look at this patch, since it was marked "ready for > committer", but I don't see how it can possibly be considered committable. > [...] > > Looking at

Re: Conflict handling for COPY FROM

2020-04-08 Thread David Steele
On Fri, Mar 27, 2020 at 3:27 PM Tom Lane > wrote: I took a quick look at this patch, since it was marked "ready for committer", but I don't see how it can possibly be considered committable. Based on Tom's review I've marked this patch Returned with

Re: Conflict handling for COPY FROM

2020-03-30 Thread Surafel Temesgen
On Fri, Mar 27, 2020 at 3:27 PM Tom Lane wrote: > Surafel Temesgen writes: > > [ conflict-handling-copy-from-v16.patch ] > > I took a quick look at this patch, since it was marked "ready for > committer", but I don't see how it can possibly be considered committable. > > 1. Covering only the

Re: Conflict handling for COPY FROM

2020-03-27 Thread Tom Lane
Surafel Temesgen writes: > [ conflict-handling-copy-from-v16.patch ] I took a quick look at this patch, since it was marked "ready for committer", but I don't see how it can possibly be considered committable. 1. Covering only the errors that are thrown in DoCopy itself doesn't seem to me to

RE: Conflict handling for COPY FROM

2020-03-26 Thread asaba.takan...@fujitsu.com
Hello Surafel, From: Surafel Temesgen >An error that can be surly handled without transaction rollback can >be included in error handling but i will like to proceed without binary file >errors handling for the time being Thank you. Also it seems that you apply Alexey's comment. So I'll mark

Re: Conflict handling for COPY FROM

2020-03-26 Thread Surafel Temesgen
Hi Takanori Asaba, > > > Although it is a small point, it may be better like this: > +70005 27 36 46 56 -> 70005 27 37 47 57 > > done > I want to discuss about copy from binary file. > It seems that this patch tries to avoid the error that number of field is >

RE: Conflict handling for COPY FROM

2020-03-12 Thread asaba.takan...@fujitsu.com
Hello Surafel, From: Surafel Temesgen >>On Fri, Mar 6, 2020 at 11:30 AM mailto:asaba.takan...@fujitsu.com >> wrote: >>I think we need regression test that constraint violating row is returned >>back to the caller. >>How about this? > >okay attached is a

Re: Conflict handling for COPY FROM

2020-03-10 Thread Alexey Kondratov
On 09.03.2020 15:34, Surafel Temesgen wrote: okay attached is a rebased patch with it +    Portal        portal = NULL; ... +        portal = GetPortalByName(""); +        SetRemoteDestReceiverParams(dest, portal); I think that you do not need this, since you are using a ready

Re: Conflict handling for COPY FROM

2020-03-09 Thread Surafel Temesgen
On Fri, Mar 6, 2020 at 11:30 AM asaba.takan...@fujitsu.com < asaba.takan...@fujitsu.com> wrote: > Hello Surafel, > > Sorry for my late reply. > > From: Surafel Temesgen > >On Thu, Dec 12, 2019 at 7:51 AM mailto:asaba.takan...@fujitsu.com > wrote: > >>2. I have

RE: Conflict handling for COPY FROM

2020-03-06 Thread asaba.takan...@fujitsu.com
Hello Surafel, Sorry for my late reply. From: Surafel Temesgen >On Thu, Dec 12, 2019 at 7:51 AM mailto:asaba.takan...@fujitsu.com > wrote: >>2. I have a question about copy meta-command. >>When I executed copy meta-command, output wasn't displayed. >>Does it

Re: Conflict handling for COPY FROM

2020-02-17 Thread Surafel Temesgen
On Mon, Feb 17, 2020 at 10:00 AM Tatsuo Ishii wrote: > >> test=# copy t1 from '/tmp/a' with (error_limit 1); > >> ERROR: duplicate key value violates unique constraint "t1_pkey" > >> DETAIL: Key (i)=(2) already exists. > >> CONTEXT: COPY t1, line 2: "2 2" > >> > >> So if the number of

Re: Conflict handling for COPY FROM

2020-02-16 Thread Tatsuo Ishii
>> test=# copy t1 from '/tmp/a' with (error_limit 1); >> ERROR: duplicate key value violates unique constraint "t1_pkey" >> DETAIL: Key (i)=(2) already exists. >> CONTEXT: COPY t1, line 2: "2 2" >> >> So if the number of errors raised exceeds error_limit, no constaraint >> violating rows (in

Re: Conflict handling for COPY FROM

2020-02-16 Thread Surafel Temesgen
Hi, > > ERROR_LIMIT ' class="parameter">limit_number' > > > > I think this should be: > > > > ERROR_LIMIT limit_number > > > > (no single quote) > > Thank you .Fixed > More comments: > > - I think the document should stat that if limit_number = 0, all > errors are immediately raised

Re: Conflict handling for COPY FROM

2020-02-16 Thread Tatsuo Ishii
> In your patch for copy.sgml: > > ERROR_LIMIT 'limit_number' > > I think this should be: > > ERROR_LIMIT limit_number > > (no single quote) More comments: - I think the document should stat that if limit_number = 0, all errors are immediately raised (behaves same as current

Re: Conflict handling for COPY FROM

2020-02-13 Thread Tatsuo Ishii
In your patch for copy.sgml: ERROR_LIMIT 'limit_number' I think this should be: ERROR_LIMIT limit_number (no single quote) Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp

Re: Conflict handling for COPY FROM

2019-12-16 Thread Surafel Temesgen
On Thu, Dec 12, 2019 at 7:51 AM asaba.takan...@fujitsu.com < asaba.takan...@fujitsu.com> wrote: > 2. I have a question about copy meta-command. > When I executed copy meta-command, output wasn't displayed. > Does it correspond to copy meta-command? > > Fixed regards Surafel diff --git

Re: Conflict handling for COPY FROM

2019-12-12 Thread Surafel Temesgen
Hi Asaba, On Thu, Dec 12, 2019 at 7:51 AM asaba.takan...@fujitsu.com < asaba.takan...@fujitsu.com> wrote: > Hello Surafel, > > I'm very interested in this patch. > Although I'm a beginner,I would like to participate in the development of > PostgreSQL. > > > 1. I want to suggest new output

RE: Conflict handling for COPY FROM

2019-12-11 Thread asaba.takan...@fujitsu.com
Hello Surafel, I'm very interested in this patch. Although I'm a beginner,I would like to participate in the development of PostgreSQL. 1. I want to suggest new output format. In my opinion, it's kind to display description of output and add "line number" and "error" to output. For example,

Re: Conflict handling for COPY FROM

2019-11-24 Thread Surafel Temesgen
On Thu, Nov 21, 2019 at 4:22 PM Alexey Kondratov wrote: > > Now the whole patch works exactly as expected for me and I cannot find > any new technical flaws. However, the doc is rather vague, especially > these places: > > + specifying it to -1 returns all error record. > > Actually, we

Re: Conflict handling for COPY FROM

2019-11-21 Thread Alexey Kondratov
On 18.11.2019 9:42, Surafel Temesgen wrote: On Fri, Nov 15, 2019 at 6:24 PM Alexey Kondratov mailto:a.kondra...@postgrespro.ru>> wrote: 1) Maybe it is fine, but now I do not like this part: +    portal = GetPortalByName(""); +    dest = CreateDestReceiver(DestRemote); +   

Re: Conflict handling for COPY FROM

2019-11-17 Thread Surafel Temesgen
On Fri, Nov 15, 2019 at 6:24 PM Alexey Kondratov wrote: > On 11.11.2019 16:00, Surafel Temesgen wrote: > > > > > > Next, you use DestRemoteSimple for returning conflicting tuples back: > > > > +dest = CreateDestReceiver(DestRemoteSimple); > > +dest->rStartup(dest,

Re: Conflict handling for COPY FROM

2019-11-15 Thread Alexey Kondratov
On 11.11.2019 16:00, Surafel Temesgen wrote: Next, you use DestRemoteSimple for returning conflicting tuples back: +        dest = CreateDestReceiver(DestRemoteSimple); +        dest->rStartup(dest, (int) CMD_SELECT, tupDesc); However, printsimple supports very limited subset

Re: Conflict handling for COPY FROM

2019-11-11 Thread Surafel Temesgen
On Fri, Sep 20, 2019 at 4:16 PM Alexey Kondratov wrote: > > First of all, there is definitely a problem with grammar. In docs ERROR > is defined as option and > > COPY test FROM '/path/to/copy-test-simple.csv' ERROR -1; > > works just fine, but if modern 'WITH (...)' syntax is used: > > COPY

Re: Conflict handling for COPY FROM

2019-09-20 Thread Alexey Kondratov
Hi Surafel, On 16.07.2019 10:08, Surafel Temesgen wrote: i also add an option to ignore all errors in ERROR set to -1 Great! The patch still applies cleanly (tested on e1c8743e6c), but I've got some problems using more elaborated tests. First of all, there is definitely a problem with

Re: Conflict handling for COPY FROM

2019-07-16 Thread Surafel Temesgen
On Sun, Jul 14, 2019 at 7:40 PM Alvaro Herrera wrote: > > error_limit being an integer, please don't use it as a boolean: > > if (cstate->error_limit) > ... > > Add an explicit comparison to zero instead, for code readability. > Also, since each error decrements the same variable, it

Re: Conflict handling for COPY FROM

2019-07-14 Thread Alvaro Herrera
I think making ERROR a reserved word is a terrible idea, and we don't need it for this feature anyway. Use a new option in the parenthesized options list instead. error_limit being an integer, please don't use it as a boolean: if (cstate->error_limit) ... Add an explicit comparison to

Re: Conflict handling for COPY FROM

2019-07-14 Thread Anthony Nowocien
Hi, sorry for answering a bit later than I hoped. Here is my review so far: Contents == This patch starts to address in my opinion one of COPY's shortcoming, which is error handling. PK and exclusion errors are taken care of, but not (yet?) other types of errors. Documentation is

Re: Conflict handling for COPY FROM

2019-07-13 Thread Thomas Munro
On Fri, Jul 12, 2019 at 1:42 AM Surafel Temesgen wrote: > Here are the patch that contain all the comment given except adding a way to > specify > to ignoring all error because specifying a highest number can do the work and > may be > try to store such badly structure data is a bad idea Hi

Re: Conflict handling for COPY FROM

2019-07-11 Thread Surafel Temesgen
Hi > > Also, I would prefer having an option to ignore all errors, e.g. with > option ERROR_LIMIT set to -1. Because it is rather difficult to estimate > a number of future errors if you are playing with some badly structured > data, while always setting it to 100500k looks ugly. > Here are the

Re: Conflict handling for COPY FROM

2019-07-04 Thread Thomas Munro
On Fri, Jun 28, 2019 at 10:57 PM Surafel Temesgen wrote: > In addition to the above change in the attached patch i also change > the syntax to ERROR LIMIT because it is no longer only skip > unique and exclusion constrain violation Hi Surafel, FYI copy.sgml has some DTD validity problems.

Re: Conflict handling for COPY FROM

2019-07-03 Thread Anthony Nowocien
Hi, I'm very interested in this patch and would like to give a review within a week. On the feature side, how about simply using the less verbose "ERRORS" instead of "ERROR LIMIT" ? On Wed, Jul 3, 2019 at 1:42 PM Surafel Temesgen wrote: > Hi Alexey, > Thank you for looking at it > > On Tue, Jul

Re: Conflict handling for COPY FROM

2019-07-03 Thread Surafel Temesgen
Hi Alexey, Thank you for looking at it On Tue, Jul 2, 2019 at 7:57 PM Alexey Kondratov wrote: > On 28.06.2019 16:12, Alvaro Herrera wrote: > >> On Wed, Feb 20, 2019 at 7:04 PM Andres Freund > wrote: > >>> Or even just return it as a row. CopyBoth is relatively widely > supported > >>> these

Re: Conflict handling for COPY FROM

2019-07-02 Thread Alexey Kondratov
On 28.06.2019 16:12, Alvaro Herrera wrote: On Wed, Feb 20, 2019 at 7:04 PM Andres Freund wrote: Or even just return it as a row. CopyBoth is relatively widely supported these days. i think generating warning about it also sufficiently meet its propose of notifying user about skipped record

Re: Conflict handling for COPY FROM

2019-06-28 Thread Alvaro Herrera
On 2019-Jun-28, Surafel Temesgen wrote: > On Wed, Feb 20, 2019 at 7:04 PM Andres Freund wrote: > > On February 20, 2019 6:05:53 AM PST, Andrew Dunstan < > > andrew.duns...@2ndquadrant.com> wrote: > > >Why log to a file at all? We do have, you know, a database handy, where > > >we might more

Re: Conflict handling for COPY FROM

2019-06-28 Thread Surafel Temesgen
On Wed, Feb 20, 2019 at 7:04 PM Andres Freund wrote: > > > On February 20, 2019 6:05:53 AM PST, Andrew Dunstan < > andrew.duns...@2ndquadrant.com> wrote: > > > >On 2/20/19 8:01 AM, Surafel Temesgen wrote: > >> > >> > >> On Tue, Feb 19, 2019 at 3:47 PM Andres Freund >>

Re: Conflict handling for COPY FROM

2019-04-03 Thread Andres Freund
Hi, On 2019-03-25 12:50:13 +0400, David Steele wrote: > On 2/20/19 8:03 PM, Andres Freund wrote: > > On February 20, 2019 6:05:53 AM PST, Andrew Dunstan > > wrote: > > > > > > Why log to a file at all? We do have, you know, a database handy, where > > > we might more usefully log errors. You

Re: Re: Conflict handling for COPY FROM

2019-03-25 Thread David Steele
Hi Surafel, On 2/20/19 8:03 PM, Andres Freund wrote: On February 20, 2019 6:05:53 AM PST, Andrew Dunstan wrote: Why log to a file at all? We do have, you know, a database handy, where we might more usefully log errors. You could usefully log the offending row as an array of text, possibly.

Re: Conflict handling for COPY FROM

2019-02-20 Thread Andres Freund
On February 20, 2019 6:05:53 AM PST, Andrew Dunstan wrote: > >On 2/20/19 8:01 AM, Surafel Temesgen wrote: >> >> >> On Tue, Feb 19, 2019 at 3:47 PM Andres Freund > > wrote: >> >> >> >> Err, what? Again, that requires super user permissions (in >> contrast to

Re: Conflict handling for COPY FROM

2019-02-20 Thread Andrew Dunstan
On 2/20/19 8:01 AM, Surafel Temesgen wrote: > > > On Tue, Feb 19, 2019 at 3:47 PM Andres Freund > wrote: > > > > Err, what? Again, that requires super user permissions (in > contrast to copy from/to stdin/out). Backends run as the user > postgres runs

Re: Conflict handling for COPY FROM

2019-02-20 Thread Surafel Temesgen
On Tue, Feb 19, 2019 at 3:47 PM Andres Freund wrote: > > > Err, what? Again, that requires super user permissions (in contrast to > copy from/to stdin/out). Backends run as the user postgres runs under > okay i see it now and modified the patch similarly regards Surafel diff --git

Re: Conflict handling for COPY FROM

2019-02-19 Thread Andres Freund
On February 19, 2019 3:05:37 AM PST, Surafel Temesgen wrote: >On Sat, Feb 16, 2019 at 8:24 AM Andres Freund >wrote: > >> Hi, >> >> On 2018-08-23 17:11:04 +0300, Surafel Temesgen wrote: >> > COPY ... WITH ON CONFLICT LOG maximum_error, LOG FILE NAME '…'; >> >> This doesn't seem to address

Re: Conflict handling for COPY FROM

2019-02-19 Thread Surafel Temesgen
On Sat, Feb 16, 2019 at 8:24 AM Andres Freund wrote: > Hi, > > On 2018-08-23 17:11:04 +0300, Surafel Temesgen wrote: > > COPY ... WITH ON CONFLICT LOG maximum_error, LOG FILE NAME '…'; > > This doesn't seem to address Robert's point that a log file requires to > be super user only, which seems

Re: Conflict handling for COPY FROM

2019-02-19 Thread Surafel Temesgen
On Mon, Feb 4, 2019 at 9:06 AM Michael Paquier wrote: > On Wed, Dec 19, 2018 at 02:48:14PM +0300, Surafel Temesgen wrote: > > Thank you for informing, attach is rebased patch against current > > master > > copy.c conflicts on HEAD, please rebase. I am moving the patch to > next CF, waiting on

Re: Conflict handling for COPY FROM

2019-02-17 Thread Andrew Dunstan
On 2/16/19 12:24 AM, Andres Freund wrote: > Hi, > > On 2018-08-23 17:11:04 +0300, Surafel Temesgen wrote: >> COPY ... WITH ON CONFLICT LOG maximum_error, LOG FILE NAME '…'; > This doesn't seem to address Robert's point that a log file requires to > be super user only, which seems to restrict the

Re: Conflict handling for COPY FROM

2019-02-15 Thread Andres Freund
Hi, On 2018-08-23 17:11:04 +0300, Surafel Temesgen wrote: > COPY ... WITH ON CONFLICT LOG maximum_error, LOG FILE NAME '…'; This doesn't seem to address Robert's point that a log file requires to be super user only, which seems to restrict the feature more than necessary? - Andres

Re: Conflict handling for COPY FROM

2019-02-03 Thread Michael Paquier
On Wed, Dec 19, 2018 at 02:48:14PM +0300, Surafel Temesgen wrote: > Thank you for informing, attach is rebased patch against current > master copy.c conflicts on HEAD, please rebase. I am moving the patch to next CF, waiting on author. -- Michael signature.asc Description: PGP signature

Re: Conflict handling for COPY FROM

2018-12-19 Thread Surafel Temesgen
On Thu, Nov 29, 2018 at 3:15 PM Dmitry Dolgov <9erthali...@gmail.com> wrote: > > Unfortunately, the patch conflict-handling-onCopy-from-v2.patch has some > conflicts now, could you rebase it? > Thank you for informing, attach is rebased patch against current master Regards Surafel diff --git

Re: Conflict handling for COPY FROM

2018-11-29 Thread Dmitry Dolgov
> On Thu, Aug 23, 2018 at 4:16 PM Surafel Temesgen > wrote: > > The attached patch add error handling for > Extra data > > missing data > > invalid oid > > null oid and > > row count mismatch Hi, Unfortunately, the patch conflict-handling-onCopy-from-v2.patch has some conflicts now, could you

Re: Conflict handling for COPY FROM

2018-08-23 Thread Surafel Temesgen
Hello, The attached patch add error handling for Extra data missing data invalid oid null oid and row count mismatch And the record that field on the above case write to the file with appended error message in it and in case of unique violation or exclusion constraint violation error the

Re: Conflict handling for COPY FROM

2018-08-20 Thread Robert Haas
On Sat, Aug 4, 2018 at 9:10 AM Surafel Temesgen wrote: > In order to prevent extreme condition the patch also add a new GUC > variable called copy_max_error_limit that control the amount of error to > swallow before start to error and new failed record file options for copy > to write a failed

Re: Conflict handling for COPY FROM

2018-08-20 Thread Surafel Temesgen
Thanks for looking at it 1. It sounded like you added the copy_max_error_limit GUC as part of this patch to allow users to specify how many errors they want to swallow with this new functionality. The GUC didn't seem to be defined and we saw no mention of it in the code. My guess is this might

Re: Conflict handling for COPY FROM

2018-08-17 Thread Karen Huddleston
Hi Surafel, Andrew and I began reviewing your patch. It applied cleanly and seems to mostly have the functionality you describe. We did have some comments/questions. 1. It sounded like you added the copy_max_error_limit GUC as part of this patch to allow users to specify how many errors they