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-15 15:24:41
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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 of built-in
> types, so
> CREATE TABLE large_test (id integer primary key, num1 bigint, num2
> double precision);
> COPY large_test FROM '/path/to/copy-test.tsv';
> COPY large_test FROM '/path/to/copy-test.tsv' ERROR 3;
> fails with following error 'ERROR:  unsupported type OID: 701', which
> seems to be very confusing from the end user perspective. I've
> tried to
> switch to DestRemote, but couldn't figure it out quickly.
> fixed

Thanks, now it works with my tests.

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),
as it may be required by COPY now?

2) My second concern is that you use three internal flags to track
errors limit:

+    int            error_limit;    /* total number of error to ignore */
+    bool        ignore_error;    /* is ignore error specified? */
+    bool        ignore_all_error;    /* is error_limit -1 (ignore all
+                                     * specified? */

Though it seems that we can just leave error_limit as a user-defined
constant and track errors with something like errors_count. In that case
you do not need auxiliary ignore_all_error flag. But probably it is a
matter of personal choice.


Alexey Kondratov

Postgres Professional
Russian Postgres Company

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Konstantin Knizhnik 2019-11-15 15:48:01 Re: Replication & recovery_min_apply_delay
Previous Message Jesper Pedersen 2019-11-15 15:05:51 Re: Index Skip Scan