Re: Conflict handling for COPY FROM

From: Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>
To: Surafel Temesgen <surafel3000(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Conflict handling for COPY FROM
Date: 2019-11-21 13:22:29
Message-ID: 5e6abe50-d74c-291a-e294-3f56604bc3c7@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 18.11.2019 9:42, Surafel Temesgen wrote:
> On Fri, Nov 15, 2019 at 6:24 PM Alexey Kondratov
> <a(dot)kondratov(at)postgrespro(dot)ru <mailto:a(dot)kondratov(at)postgrespro(dot)ru>> wrote:
>
>
> 1) Maybe it is fine, but now I do not like this part:
>
> +    portal = GetPortalByName("");
> +    dest = CreateDestReceiver(DestRemote);
> +    SetRemoteDestReceiverParams(dest, portal);
> +    dest->rStartup(dest, (int) CMD_SELECT, tupDesc);
>
> Here you implicitly use the fact that portal with a blank name is
> always
> created in exec_simple_query before we get to this point. Next, you
> create new DestReceiver and set it to this portal, but it is also
> already created and set in the exec_simple_query.
>
> Would it be better if you just explicitly pass ready DestReceiver to
> DoCopy (similarly to how it is done for T_ExecuteStmt /
> ExecuteQuery),
>
>
> Good idea .Thank you

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 return only rows with constraint violation, but malformed
rows are ignored with warning. I guess that we simply cannot return
malformed rows back to the caller in the same way as with constraint
violation, since we cannot figure out (in general) which column
corresponds to which type if there are extra or missing columns.

+      and same record formatting error is ignored.

I can get it, but it definitely should be reworded.

What about something like this?

+   <varlistentry>
+ <term><literal>ERROR_LIMIT</literal></term>
+    <listitem>
+     <para>
+      Enables ignoring of errored out rows up to <replaceable
+      class="parameter">limit_number</replaceable>. If <replaceable
+      class="parameter">limit_number</replaceable> is set
+      to -1, then all errors will be ignored.
+     </para>
+
+     <para>
+      Currently, only unique or exclusion constraint violation
+      and rows formatting errors are ignored. Malformed
+      rows will rise warnings, while constraint violating rows
+      will be returned back to the caller.
+     </para>
+
+    </listitem>
+   </varlistentry>

Regards
--
Alexey Kondratov

Postgres Professional https://www.postgrespro.com
Russian Postgres Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2019-11-21 14:35:33 Re: TAP tests aren't using the magic words for Windows file access
Previous Message Leif Gunnar Erlandsen 2019-11-21 12:50:18 Re: pause recovery if pitr target not reached