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
error)
+ * 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.
Regards
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company
