public inbox for [email protected]  
help / color / mirror / Atom feed
From: Alena Rybakina <[email protected]>
To: Damir Belyalov <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Andrey Lepikhov <[email protected]>
Subject: Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features)
Date: Sat, 6 May 2023 23:05:04 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALH1LgseNT7mxpGzQsQwOJz_UwYBo6L-4vR9tmJqb-WNPkt6Bg@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<CALH1LgvQ5eRMi3zO7RSmod4ucru=9ekASpReahR7zDcCabANZw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CALH1LguAEsoTYJTCsXNB-7z2Hu9UGEpsXA4kj0FOTmoP=6Wp3Q@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CALH1LguOY-VvJBWvjpLR2W3t0Z2p=LGSK5tzLrKwtzZ2UsqwGg@mail.gmail.com>
	<[email protected]>
	<CALH1LgseNT7mxpGzQsQwOJz_UwYBo6L-4vR9tmJqb-WNPkt6Bg@mail.gmail.com>

Hi!

Thank you, Damir, for your patch. It is very interesting to review it!

It seemed to me that the names of variables are not the same everywhere.

I noticed that you used /ignore_datatype_errors_specified/ variable in 
/copy.c/ , but guc has a short name /ignore_datatype_errors/. Also you 
used the short variable name in /CopyFormatOptions/ structure.
Name used /ignore_datatype_errors_specified /is seemed very long to me, 
may be use a short version of it (/ignore_datatype_errors/) in /copy.c/ too?

Besides, I noticed that you used /ignored_errors/ variable in 
/CopyFromStateData/ structure and it's name is strikingly similar to 
name (/ignore_datatype_error//s/), but they have different meanings.
Maybe it will be better to rename it as /ignored_errors_counter/?


I tested last version 
/v5-0001-Add-new-COPY-option-IGNORE_DATATYPE_ERRORS.patch/ with /bytea/ 
data type and transaction cases. Eventually, I didn't find any problem 
there.
I described my steps in more detail, if I missed something.

*First of all, I ran copy function with IGNORE_DATATYPE_ERRORS parameter 
being in transaction block.*
/
//File t2.csv exists:/

|id,b
769,\
1,\6e
2,\x5
5,\x|

/Test:/
CREATE TABLE t (id INT , b  BYTEA) ;
postgres=# BEGIN;
copy t FROM '/home/alena/postgres/t2.csv'  WITH (format 'csv', 
IGNORE_DATATYPE_ERRORS, delimiter ',', HEADER);
SAVEPOINT my_savepoint;
BEGIN
WARNING:  invalid input syntax for type bytea
WARNING:  invalid input syntax for type bytea
WARNING:  invalid hexadecimal data: odd number of digits
WARNING:  3 rows were skipped due to data type incompatibility
COPY 1
SAVEPOINT
postgres=*# copy t FROM '/home/alena/postgres/t2.csv'  WITH (format 
'csv', IGNORE_DATATYPE_ERRORS, delimiter ',', HEADER);
WARNING:  invalid input syntax for type bytea
WARNING:  invalid input syntax for type bytea
WARNING:  invalid hexadecimal data: odd number of digits
WARNING:  3 rows were skipped due to data type incompatibility
COPY 1
postgres=*# ROLLBACK TO my_savepoint;
ROLLBACK
postgres=*# select * from t;
  id | b
----+----
   5 | \x
(1 row)

postgres=*# copy t FROM '/home/alena/postgres/t2.csv'  WITH (format 
'csv', IGNORE_DATATYPE_ERRORS, delimiter ',', HEADER);
WARNING:  invalid input syntax for type bytea
WARNING:  invalid input syntax for type bytea
WARNING:  invalid hexadecimal data: odd number of digits
WARNING:  3 rows were skipped due to data type incompatibility
COPY 1
postgres=*# select * from t;
  id | b
----+----
   5 | \x
   5 | \x
(2 rows)

postgres=*# commit;
COMMIT

*I tried to use the similar test and moved transaction block in function:*
CREATE FUNCTION public.log2()
  RETURNS void
  LANGUAGE plpgsql
  SECURITY DEFINER
AS $function$
BEGIN;
copy t FROM '/home/alena/postgres/t2.csv'  WITH (format 'csv', 
IGNORE_DATATYPE_ERRORS, delimiter ',', HEADER);
SAVEPOINT my_savepoint;
END;
$function$;

postgres=# delete from t;

postgres=# select 1 as t from log2();
WARNING:  invalid input syntax for type bytea
WARNING:  invalid input syntax for type bytea
WARNING:  invalid hexadecimal data: odd number of digits
WARNING:  3 rows were skipped due to data type incompatibility
  t
---
  1
(1 row)

*Secondly I checked function copy with bytea datatype. *
/t1.csv consists:/
id,b
769,\x2d
1,\x6e
2,\x5c
5,\x

/And I ran it:/

postgres=# delete from t;
DELETE 4
postgres=# copy t FROM '/home/alena/postgres/t2.csv'  WITH (format 
'csv', IGNORE_DATATYPE_ERRORS, delimiter ',', HEADER);
WARNING:  invalid input syntax for type bytea
WARNING:  invalid input syntax for type bytea
WARNING:  invalid hexadecimal data: odd number of digits
WARNING:  3 rows were skipped due to data type incompatibility
COPY 1
postgres=# select * from t;
  id | b
----+----
   5 | \x
(1 row)

-- 
---
Alena Rybakina
Postgres Professional


view thread (4+ 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