I agree. Fixed for DBI 1.57. Thanks Bart!

Tim.

On Thu, Jun 07, 2007 at 03:42:59PM +0200, Bart Degryse wrote:
> According to the perldoc for DBI.pm module version 1.56 
> (http://search.cpan.org/~timb/DBI/DBI.pm#execute_for_fetch): 
> 
> If [EMAIL PROTECTED] is passed then the execute_for_fetch method uses it to 
> return status information. The tuple_status array holds one element per 
> tuple. (...) If the execute() did fail then the element holds a reference to 
> an array containing ($sth->err, $sth->errstr, $sth->state).
> As I understand this for each tuple that fails execution @tuple_status will 
> hold an array with information on why this particular tuple failed.
> In my test scenario (see below) I'm fetching 14 records from Oracle and I try 
> to insert them into a PostgreSQL database. The insert trigger on the target 
> table will raise an exception when field [dataareaid] contains the value 
> 'lil' and another exception when it contains the value 'bol'. Both exceptions 
> will thus cause the insert to fail.
> Of those 14 records two have 'lil' and two have 'bol' as [dataareaid] value. 
> So based on my understanding of the perldoc I expect @tuple_status to hold 10 
> scalars (the return value from the records that executed), two arrays holding 
> the lil exception message and two arrays holding the bol exception message.
> That is however not what I get. In my test scenario @tuple_status holds the 
> 10 scalars and 4 arrays holding one of the exception messages (namely the one 
> that is encountered first while inserting).
> I think the code on line 1931 is causing this behaviour
>    push @$tuple_status, [ $err, $errstr_cache{$err} ||= $sth->errstr, 
> $sth->state ];
> The first exception message being raised gets cached in $errstr_cache. 
> Subsequent (different) messages that have the same $err are not cached.
> As PostgreSQL's plpgsql always generates the same SQLSTATE for a raised 
> exception 
> (http://www.postgresql.org/docs/8.2/interactive/plpgsql-errors-and-messages.html),
>  no matter what message it is invoced with, $err (native database engine 
> error code) will always be 1 and only the first message will be cached and 
> returned into @tuple_status.
> As I can see no obvious advantage in caching the messages, nor in storing the 
> value of $sth->err temporarily in $err, I propose to replace lines 1930 and 
> 1931
>   my $err = $sth->err;
>   push @$tuple_status, [ $err, $errstr_cache{$err} ||= $sth->errstr, 
> $sth->state ];
> with this one line
>   push @$tuple_status, [ $sth->err, $sth->errstr, $sth->state ];
> 
> This replacement provides the expected values in @tuple_status.
> Thanks for looking into this.
> Bart Degryse
> Senior Developer
> --------------------------------------
> My test scenario:
> This is the target table definition in PostgreSQL
> CREATE TABLE "public"."afh_test" (
>   "addrformat" VARCHAR(10) NOT NULL, 
>   "name" VARCHAR(30) NOT NULL, 
>   "dataareaid" VARCHAR(3) NOT NULL, 
>   "recid" NUMERIC(10,0) NOT NULL
> ) WITHOUT OIDS;
>  
> CREATE UNIQUE INDEX "afh_test_idx" ON "public"."afh_test"
>   USING btree ("addrformat", "dataareaid");
> The source table definition is the same as the target table definition, 
> though the syntax differs slightly:
> CREATE TABLE ADDRESSFORMATHEADING (
>   ADDRFORMAT VARCHAR2(10 BYTE) DEFAULT '.' NOT NULL,
>   NAME VARCHAR2(30 BYTE) DEFAULT '.' NOT NULL,
>   DATAAREAID VARCHAR2(3 BYTE) DEFAULT 'dat' NOT NULL,
>   RECID NUMBER(10) NOT NULL
> )
> This is the data in Oracle:
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('national', 'This country', 'ash', 30);
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('be', 'Address Belgium', 'lil', 501);
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('fr', 'Address France', 'lil', 496);
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('it', 'Italie', 'bol', 3138);
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('national', 'National', '012', 687181679);
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('internatio', 'International countries', 'ash', 29);
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('be', 'Beglie', 'bol', 3187);
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('sp', 'Address Spain', 'bar', 1302174);
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('internatio', 'International countries', 'as0', 29);
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('national', 'This country', 'as0', 30);
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('national', 'National', '011', 216774985);
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('internatio', 'International', '011', 216774984);
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('national', 'National', 'hlm', 451094066);
> Insert into addressformatheading (ADDRFORMAT, NAME, DATAAREAID, RECID) Values 
> ('internatio', 'International', 'hlm', 451094067);
> This is the trigger I added to the target table. Like it's written here 
> records with a dataareaid that doesn't trigger an error is not inserted. By 
> using RETURN NEW; in the trigger they do get inserted, but that doesn't 
> change the fact that I only get one type of error message instead of two.
> CREATE TRIGGER "afh_test_tr" BEFORE INSERT 
> ON "public"."afh_test" FOR EACH ROW 
> EXECUTE PROCEDURE "public"."temp_func1"();
>  
> CREATE OR REPLACE FUNCTION "public"."temp_func1" () RETURNS trigger AS
> $body$
> BEGIN
>   IF NEW.dataareaid = 'lil' THEN
>     RAISE EXCEPTION '% foutje %', NEW.dataareaid, NEW.name;
>   elsIF NEW.dataareaid = 'bol' THEN
>     RAISE EXCEPTION '% nog een foutje %', NEW.dataareaid, NEW.name;
>   END IF;
>   RETURN NULL;
> END;
> $body$
> LANGUAGE 'plpgsql' VOLATILE CALLED ON NULL INPUT SECURITY INVOKER;
>  
> This is the function that retrieves the Oracle data and inserts it in the 
> target table
> CREATE OR REPLACE FUNCTION "public"."dbi_insert3" () RETURNS integer AS
> $body$
>   use DBI;
>   $query = 'SELECT * FROM AddressFormatHeading';
>   $target = 'INSERT INTO afh_test (addrformat, name, dataareaid, recid) 
> VALUES (?,?,?,?)';
>  
>   my $dbh_ora = 
> DBI->connect('dbi:Oracle:database=bmssa;host=firev120-1.indicator.be;sid=mars',
>  'bmssa', '********')
>                 or elog(ERROR, "Couldn't connect to database: " . 
> DBI->errstr);
>   my $dbh_pg = 
> DBI->connect('dbi:Pg:dbname=defrevdev;host=10.100.1.21;port=2345', 
> 'defrevsys', '********')
>                or elog(ERROR, "Couldn't connect to database: " . DBI->errstr);
>  
>   my $sel = $dbh_ora->prepare($query)
>             or elog(ERROR, "Couldn't prepare statement: " . $dbh_ora->errstr);
>   $sel->execute;
>   my $ins = $dbh_pg->prepare($target)
>             or elog(ERROR, "Couldn't prepare statement: " . $dbh_pg->errstr);
>   my $fetch_tuple_sub = sub { $sel->fetchrow_arrayref };
>   my @tuple_status;
>   my $rc = $ins->execute_for_fetch($fetch_tuple_sub, [EMAIL PROTECTED]);
>   if ($dbh_pg->err) {
>     my @errors = grep { ref $_ } @tuple_status;
>     foreach my $error (@errors) {
>       elog(INFO, $error->[0] . ' - ' . $error->[1]);
>     }
>   }
>   $dbh_ora->disconnect;
>   $dbh_pg->disconnect;
> $body$
> LANGUAGE 'plperlu' VOLATILE CALLED ON NULL INPUT SECURITY INVOKER;
> To call the function:
> SELECT dbi_insert3();
> Alternatively this can be done outside PostgreSQL:
> #!/usr/bin/perl
>   use DBI;
>   $query = "SELECT * FROM AddressFormatHeading";
>   $target = 'INSERT INTO afh_test (addrformat, name, dataareaid, recid) 
> VALUES (?,?,?,?)';
>   my $dbh_ora = 
> DBI->connect('dbi:Oracle:database=bmssa;host=firev120-1.indicator.be;sid=mars',
>  'bmssa', '********')
>                 or die "Couldn't connect to database: " . DBI->errstr;
>   my $dbh_pg = 
> DBI->connect('dbi:PgPP:dbname=defrevdev;host=10.100.1.21;port=2345', 
> 'defrevsys', '********')
>                or die "Couldn't connect to database: " . DBI->errstr;
>   my $sel = $dbh_ora->prepare($query)
>             or die "Couldn't prepare statement: " . $dbh_ora->errstr;
>   $sel->execute;
>   my $ins = $dbh_pg->prepare($target)
>             or die "Couldn't prepare statement: " . $dbh_pg->errstr;
>   my $fetch_tuple_sub = sub { $sel->fetchrow_arrayref };
>   my @tuple_status;
>   my $rc = $ins->execute_for_fetch($fetch_tuple_sub, [EMAIL PROTECTED]);
>   if ($dbh_pg->err) {
>     my @errors = grep { ref $_ } @tuple_status;
>     foreach my $error (@errors) {
>       print $error->[0].' - '.$error->[1];
>     }
>   }
>   $dbh_ora->disconnect;
>   $dbh_pg->disconnect;
> 
> --------------------------------------
> Output of the script outside PostgreSQL without my proposed patch:
> 1 - ERROR:  lil foutje Address Belgium
> 1 - ERROR:  lil foutje Address Belgium
> 1 - ERROR:  lil foutje Address Belgium
> 1 - ERROR:  lil foutje Address Belgium
> Output of the script outside PostgreSQL with my proposed patch:
> 1 - ERROR:  lil foutje Address Belgium
> 1 - ERROR:  lil foutje Address France
> 1 - ERROR:  bol nog een foutje Italie
> 1 - ERROR:  bol nog een foutje Beglie
>  

Reply via email to