Re: execute_array/execute_for_fetch (was: ANNOUNCE: Advanced DBI Tutorial Slides)
On Tue, Sep 05, 2006 at 09:13:04PM +0100, Martin J. Evans wrote: > On Tue, 2006-09-05 at 20:36 +0100, Tim Bunce wrote: > > > > @@ -1921,10 +1921,12 @@ > > # start with empty status array > > ($tuple_status) ? @$tuple_status = () : $tuple_status = []; > > > > + my $rc_total = 0; > > my ($err_count, %errstr_cache); > > while ( my $tuple = &$fetch_tuple_sub() ) { > > if ( my $rc = $sth->execute(@$tuple) ) { > > push @$tuple_status, $rc; > > + $rc_total += $rc; > > } > > else { > > $err_count++; > > @@ -1935,7 +1937,9 @@ > > my $tuples = @$tuple_status; > > return $sth->set_err(1, "executing $tuples generated $err_count > > errors") > > if $err_count; > > - return scalar(@$tuple_status) || "0E0"; > > + $tuples ||= "0E0"; > > + return $tuples unless wantarray; > > + return ($tuples, $rc_total); > > } > Yes, obvious, I was thinking from the app side (where ArrayTupleStatus > would be required) instead of the DBI side which already has the result > from execute. > > > Could you check it, add any polish needed, plus some docs (noting > > Ronald's clarification) and, ideally, some tests? > > Will do although how does this affect the DBDs that implement > execute_array themselves? e.g. Oracle. The docs just need to say that in list context some drivers may supply and extra return value giving the sum of the rows affected for each tuple. > Given Xho's comment it would also seem wise to add to the docs that > ArrayTupleStatus may not contain the actual affected rows for some > drivers e.g. Oracle. True. (Though I didn't see Xho's comment) > Also, given xho's comments that the bulk method in DBD::Oracle does not > know the affected rows (and hence returns -1 affected rows) That's a great shame if it's true. Seems odd though. > the actual change will be of no use to me (I will have to drop using > execute_array since I need to be sure the number of rows I intend to > insert or update are updated). Perhaps bulk insert to a temp table then run a stored procedure to perform the required operation and return the rows affected. > In my experience, running an update which does not affect > any rows when the intention was to update a row is usually a serious > error so it is a shame execute_array won't provide this but if OCI can't > do it, it can't. I agree. Perhaps there is a way but I don't have time to research the OCI docs. It does seem like an odd ommision. > I will amend my rt.cpan.org bug report as the main issue was returning > -1 in rows affected. > > > If you don't have commit access to the repository yet then send me your > > auth.perl.org (or bitcard.org) username and I'll give you access so you > > can commit it yourself. > > I was wondering how long I could hold out after your request for > assistance on DBI 1 - it would seem I managed less than a week ;-) :) > My username is mjevans. You should have write access shortly. Welcome! And, thanks! Tim.
Re: execute_array/execute_for_fetch (was: ANNOUNCE: Advanced DBI Tutorial Slides)
On Tue, 2006-09-05 at 20:36 +0100, Tim Bunce wrote: > On Tue, Sep 05, 2006 at 02:10:24PM +0100, Martin J. Evans wrote: > > > > > > The DBIs default behaviour is the official behaviour. > > > > That is fair enough. I wasn't really suggesting it was changed, it was more > > of > > a pointing out the inconvenience in case it led to a better way for me to > > get > > the rows affected... > > > > > I take your point about the return value. I remember weighing up both > > > alternatives before settling on 'tuples executed' - but I can't now > > > remember why! I probably also didn't consider bulk updates as a likely > > > use case. > > > > > > How about adding total rows affected as an extra return value in list > > > context?: > > > > > > ($tuples_executed, $rows_affected) = $sth->execute_array(...) > > > > and it did. Yes, this would be ideal and second guessing you I even looked > > at > > the code to implement it but was slightly flumaxed by the possible > > definition > > changes to "Dynamically create the DBI Standard Interface" > > I'm not sure what you mean. When I had a quick look it seemed like it maybe needed a change to stipulate execute_array could return an array or a scalar. Given your comment it would seem I was wrong. > > and the > > problem that it can only be calculated if ArrayTupleStatus is specified and > > that is optional now. Any pointers on these welcome and I'll attempt a > > patch. > > Here's a draft patch... (since it was trivial to do while I was > thinking about it): > > @@ -1921,10 +1921,12 @@ > # start with empty status array > ($tuple_status) ? @$tuple_status = () : $tuple_status = []; > > + my $rc_total = 0; > my ($err_count, %errstr_cache); > while ( my $tuple = &$fetch_tuple_sub() ) { > if ( my $rc = $sth->execute(@$tuple) ) { > push @$tuple_status, $rc; > + $rc_total += $rc; > } > else { > $err_count++; > @@ -1935,7 +1937,9 @@ > my $tuples = @$tuple_status; > return $sth->set_err(1, "executing $tuples generated $err_count > errors") > if $err_count; > - return scalar(@$tuple_status) || "0E0"; > + $tuples ||= "0E0"; > + return $tuples unless wantarray; > + return ($tuples, $rc_total); > } Yes, obvious, I was thinking from the app side (where ArrayTupleStatus would be required) instead of the DBI side which already has the result from execute. > Could you check it, add any polish needed, plus some docs (noting > Ronald's clarification) and, ideally, some tests? Will do although how does this affect the DBDs that implement execute_array themselves? e.g. Oracle. Given Xho's comment it would also seem wise to add to the docs that ArrayTupleStatus may not contain the actual affected rows for some drivers e.g. Oracle. Also, given xho's comments that the bulk method in DBD::Oracle does not know the affected rows (and hence returns -1 affected rows) the actual change will be of no use to me (I will have to drop using execute_array since I need to be sure the number of rows I intend to insert or update are updated). In my experience, running an update which does not affect any rows when the intention was to update a row is usually a serious error so it is a shame execute_array won't provide this but if OCI can't do it, it can't. I will amend my rt.cpan.org bug report as the main issue was returning -1 in rows affected. > If you don't have commit access to the repository yet then send me your > auth.perl.org (or bitcard.org) username and I'll give you access so you > can commit it yourself. I was wondering how long I could hold out after your request for assistance on DBI 1 - it would seem I managed less than a week ;-) My username is mjevans. > Thanks Martin! > > Tim. > > Martin -- Martin J. Evans Easysoft Limited http://www.easysoft.com
Re: execute_array/execute_for_fetch (was: ANNOUNCE: Advanced DBI Tutorial Slides)
On Tue, Sep 05, 2006 at 02:10:24PM +0100, Martin J. Evans wrote: > > > > The DBIs default behaviour is the official behaviour. > > That is fair enough. I wasn't really suggesting it was changed, it was more of > a pointing out the inconvenience in case it led to a better way for me to get > the rows affected... > > > I take your point about the return value. I remember weighing up both > > alternatives before settling on 'tuples executed' - but I can't now > > remember why! I probably also didn't consider bulk updates as a likely > > use case. > > > > How about adding total rows affected as an extra return value in list > > context?: > > > > ($tuples_executed, $rows_affected) = $sth->execute_array(...) > > and it did. Yes, this would be ideal and second guessing you I even looked at > the code to implement it but was slightly flumaxed by the possible definition > changes to "Dynamically create the DBI Standard Interface" I'm not sure what you mean. > and the > problem that it can only be calculated if ArrayTupleStatus is specified and > that is optional now. Any pointers on these welcome and I'll attempt a patch. Here's a draft patch... (since it was trivial to do while I was thinking about it): @@ -1921,10 +1921,12 @@ # start with empty status array ($tuple_status) ? @$tuple_status = () : $tuple_status = []; + my $rc_total = 0; my ($err_count, %errstr_cache); while ( my $tuple = &$fetch_tuple_sub() ) { if ( my $rc = $sth->execute(@$tuple) ) { push @$tuple_status, $rc; + $rc_total += $rc; } else { $err_count++; @@ -1935,7 +1937,9 @@ my $tuples = @$tuple_status; return $sth->set_err(1, "executing $tuples generated $err_count errors") if $err_count; - return scalar(@$tuple_status) || "0E0"; + $tuples ||= "0E0"; + return $tuples unless wantarray; + return ($tuples, $rc_total); } Could you check it, add any polish needed, plus some docs (noting Ronald's clarification) and, ideally, some tests? If you don't have commit access to the repository yet then send me your auth.perl.org (or bitcard.org) username and I'll give you access so you can commit it yourself. Thanks Martin! Tim.
Re: execute_array/execute_for_fetch (was: ANNOUNCE: Advanced DBI Tutorial Slides)
On 05-Sep-2006 Martin J. Evans wrote: > > On 05-Sep-2006 Tim Bunce wrote: >> On Tue, Sep 05, 2006 at 11:20:04AM +0100, Martin J. Evans wrote: >>> >>> I'll report the problems just as soon as I'm certain what they are. Here is >>> my summary: >>> >>> 1. insert into table values (?,?) where all values are valid and 3 rows of >>>values >>> >>> returns: 3 >>>(correct as 3 tuples executed) >>> ArrayTupleStatus = (-1, -1, -1) >>>(incorrect - should be (1,1,1) as DBD::Oracle does know a row was >>> inserted each time as evidenced by execute itself which returns >>> rows >>> affected) >>> >>> 2. insert into table values (?,?) where there are 2 rows of data but second >>> one >>>is invalid >>> >>>returns: undef >>> (correct) >>>ArrayTupleStatus = (-1, [1772, 'ORA-01722...']) >>> (incorrect as this should be (1, [1772, 'ORA-01722...']) >>> probably same issue as 1). >>> >>> 3. update table where the update does not affect any rows >>> >>>returns: 0 >>> (incorrect - should be 1 for 1 tuple executed) >>>ArrayTupleStatus = (-1) >>> (incorrect - should be (0) for no rows affected) >>> >>> If my conclusions look right to you I'll report them on rt. >> >> You're spot on. > > OK, will report these issue this afternoon. > Reported at http://rt.cpan.org/Ticket/Display.html?id=21331 Martin -- Martin J. Evans Easysoft Ltd, UK http://www.easysoft.com
RE: execute_array/execute_for_fetch (was: ANNOUNCE: Advanced DBI Tutorial Slides)
Tim Bunce [mailto:[EMAIL PROTECTED] wrote: > How about adding total rows affected as an extra return value in list > context?: > > ($tuples_executed, $rows_affected) = $sth->execute_array(...) How do you determine total rows affected? Is it simply the sum of rows affected for each tuple? That may not be the same as the actual number of rows affected, because a single row could be affected by multiple tuples. But I think that would be okay as long it's clear in the documentation. Ronald
Re: execute_array/execute_for_fetch (was: ANNOUNCE: Advanced DBI Tutorial Slides)
On 05-Sep-2006 Tim Bunce wrote: > On Tue, Sep 05, 2006 at 11:20:04AM +0100, Martin J. Evans wrote: >> >> I'll report the problems just as soon as I'm certain what they are. Here is >> my summary: >> >> 1. insert into table values (?,?) where all values are valid and 3 rows of >>values >> >> returns: 3 >>(correct as 3 tuples executed) >> ArrayTupleStatus = (-1, -1, -1) >>(incorrect - should be (1,1,1) as DBD::Oracle does know a row was >> inserted each time as evidenced by execute itself which returns rows >> affected) >> >> 2. insert into table values (?,?) where there are 2 rows of data but second >> one >>is invalid >> >>returns: undef >> (correct) >>ArrayTupleStatus = (-1, [1772, 'ORA-01722...']) >> (incorrect as this should be (1, [1772, 'ORA-01722...']) >> probably same issue as 1). >> >> 3. update table where the update does not affect any rows >> >>returns: 0 >> (incorrect - should be 1 for 1 tuple executed) >>ArrayTupleStatus = (-1) >> (incorrect - should be (0) for no rows affected) >> >> If my conclusions look right to you I'll report them on rt. > > You're spot on. OK, will report these issue this afternoon. >> As an aside, I am finding execute_array returning the number of tuples >> executed >> rather than the number of rows affected a bit of a pain. It seems to make it >> easy to ascertain how many tuples were executed but hard to find out how >> many rows were affected when I would have thought the former is less useful >> and easily calculated as [EMAIL PROTECTED]). What I normally do on a >> single insert/update execute is: >> >> $affected = $sth->execute(@params) >> ERROR if ($affected != $expected_inserts_or_updates); >> >> as this catches errors in the execute and error in the program logic (not >> inserting/updating what I expected). execute_array does not map easily to >> being >> n * execute as far as I can see since the equivalent test for execute_array >> is: >> >> $tuples_executed = $sth->execute_array >> if (!$tuples_executed) { >> # there was an error >> } else { >> # here $tuples_executed has no meaning for me since I know what it is >> # going >> # to be before calling execute_array - it is going to be the number of >> # tuples >> # I passed to execute_array >> my $affected = 0; >> $affected += $_ foreach (@array_tuple_status); >> ERROR if ($affected != $expected_inserts_or_updates) >> } >> >> It's possible I've misunderstood what is returned by execute_array but it is >> difficult to actually try it out when I don't seem to have a driver that >> does >> it properly. > > The DBIs default behaviour is the official behaviour. That is fair enough. I wasn't really suggesting it was changed, it was more of a pointing out the inconvenience in case it led to a better way for me to get the rows affected... > I take your point about the return value. I remember weighing up both > alternatives before settling on 'tuples executed' - but I can't now > remember why! I probably also didn't consider bulk updates as a likely > use case. > > How about adding total rows affected as an extra return value in list > context?: > > ($tuples_executed, $rows_affected) = $sth->execute_array(...) > > Tim. and it did. Yes, this would be ideal and second guessing you I even looked at the code to implement it but was slightly flumaxed by the possible definition changes to "Dynamically create the DBI Standard Interface" and the problem that it can only be calculated if ArrayTupleStatus is specified and that is optional now. Any pointers on these welcome and I'll attempt a patch. Martin -- Martin J. Evans Easysoft Ltd, UK http://www.easysoft.com
Re: execute_array/execute_for_fetch (was: ANNOUNCE: Advanced DBI Tutorial Slides)
On Tue, Sep 05, 2006 at 11:20:04AM +0100, Martin J. Evans wrote: > > I'll report the problems just as soon as I'm certain what they are. Here is > my summary: > > 1. insert into table values (?,?) where all values are valid and 3 rows of >values > > returns: 3 >(correct as 3 tuples executed) > ArrayTupleStatus = (-1, -1, -1) >(incorrect - should be (1,1,1) as DBD::Oracle does know a row was > inserted each time as evidenced by execute itself which returns rows > affected) > > 2. insert into table values (?,?) where there are 2 rows of data but second > one >is invalid > >returns: undef > (correct) >ArrayTupleStatus = (-1, [1772, 'ORA-01722...']) > (incorrect as this should be (1, [1772, 'ORA-01722...']) > probably same issue as 1). > > 3. update table where the update does not affect any rows > >returns: 0 > (incorrect - should be 1 for 1 tuple executed) >ArrayTupleStatus = (-1) > (incorrect - should be (0) for no rows affected) > > If my conclusions look right to you I'll report them on rt. You're spot on. > As an aside, I am finding execute_array returning the number of tuples > executed > rather than the number of rows affected a bit of a pain. It seems to make it > easy to ascertain how many tuples were executed but hard to find out how > many rows were affected when I would have thought the former is less useful > and easily calculated as [EMAIL PROTECTED]). What I normally do on a > single insert/update execute is: > > $affected = $sth->execute(@params) > ERROR if ($affected != $expected_inserts_or_updates); > > as this catches errors in the execute and error in the program logic (not > inserting/updating what I expected). execute_array does not map easily to > being > n * execute as far as I can see since the equivalent test for execute_array > is: > > $tuples_executed = $sth->execute_array > if (!$tuples_executed) { > # there was an error > } else { > # here $tuples_executed has no meaning for me since I know what it is going > # to be before calling execute_array - it is going to be the number of > tuples > # I passed to execute_array > my $affected = 0; > $affected += $_ foreach (@array_tuple_status); > ERROR if ($affected != $expected_inserts_or_updates) > } > > It's possible I've misunderstood what is returned by execute_array but it is > difficult to actually try it out when I don't seem to have a driver that does > it properly. The DBIs default behaviour is the official behaviour. I take your point about the return value. I remember weighing up both alternatives before settling on 'tuples executed' - but I can't now remember why! I probably also didn't consider bulk updates as a likely use case. How about adding total rows affected as an extra return value in list context?: ($tuples_executed, $rows_affected) = $sth->execute_array(...) Tim.
RE: execute_array/execute_for_fetch (was: ANNOUNCE: Advanced DBI Tutorial Slides)
On 04-Sep-2006 Tim Bunce wrote: > On Mon, Sep 04, 2006 at 05:21:17PM +0100, Martin J. Evans wrote: >> >> It did raise an issue where I could do with some clarification. The slides >> say: >> >> Execute a statement for multiple values >>$sth = $dbh->prepare("insert into table (foo,bar) values (?,?)"); >>$tuples = $sth->execute_array(\%attr, [EMAIL PROTECTED], [EMAIL >> PROTECTED]); >> >> returns count of executions (even ones that failed) and not rows-affected. >> >> but the latest DBI pod says: >> >> The execute_array() method returns the number of tuples executed, >> or "undef" if an error occured. Like execute(), a successful exe- >> cute_array() always returns true regardless of the number of tuples >> executed, even if it's zero. See the "ArrayTupleStatus" attribute >> below for how to determine the execution status for each tuple. >> >> I read that latter as if undef is returned 1 or more of the executes failed >> but >> now I read in the slides they suggest I may get a true value when an error >> occurs. My experience with execute_array and Oracle shows I get: >> >> return 0, when none resulted in a change but no errors >> e.g. >> drop table mytest >> create table mytest (a int primary key, b char(20)) >> insert into mytest values (1,'onetwothree') >> update mytest set b = ? where a = ? >> $sth->bind_param_array(2, [2]) >> $sth->bind_param_array(1, ['two']) >> $sth->execute_array( { ArrayTupleStatus => [EMAIL PROTECTED] } ) >> >> returns 0 >> >> No, error, just no update occurred. It does not return 1 as the slide >> suggests and does seem to be the number of rows changed - none. >> >> returns undef when there is an error >> e.g. >> as per above example but the bind_param_array for p2 is invalid e.g. 'xxx' >> >> This seems to also contradict the slide but agrees with the DBI pod. >> >> lastly I get undef if there are multiple rows but the last one errors. >> e.g. as above except the bind_param_arrays are: >> >> $sth->bind_param_array(2, [1, 'xxx']); >> $sth->bind_param_array(1, ['one', 'two']); >> >> returns undef but executes the 1,'one'. >> >> I am now unclear as to what is correct. Can you clarify? > > The pod is right and both the slides and DBD::Oracle are wrong, > though in different ways. > > The behaviour should match that of the default execute_for_fetch > method (which execute_array calls) provided by the DBI - shown below. > > Thanks! I've fixed my copy of the slide to say "returns count of > executions, not rows-affected, or undef if any failed". > > Tim. > > p.s. I'd be grateful if you could file a bug at rt.cpan.org for DBD::Oracle. > > > sub execute_for_fetch { > my ($sth, $fetch_tuple_sub, $tuple_status) = @_; > # start with empty status array > ($tuple_status) ? @$tuple_status = () : $tuple_status = []; > > my ($err_count, %errstr_cache); > while ( my $tuple = &$fetch_tuple_sub() ) { > if ( my $rc = $sth->execute(@$tuple) ) { > push @$tuple_status, $rc; > } > else { > $err_count++; > my $err = $sth->err; > push @$tuple_status, [ $err, $errstr_cache{$err} ||= > $sth->errstr, $sth->state ]; > } > } > my $tuples = @$tuple_status; > return $sth->set_err(1, "executing $tuples generated $err_count > errors") > if $err_count; > return scalar(@$tuple_status) || "0E0"; > } Thanks for the clarification. I'll report the problems just as soon as I'm certain what they are. Here is my summary: 1. insert into table values (?,?) where all values are valid and 3 rows of values returns: 3 (correct as 3 tuples executed) ArrayTupleStatus = (-1, -1, -1) (incorrect - should be (1,1,1) as DBD::Oracle does know a row was inserted each time as evidenced by execute itself which returns rows affected) 2. insert into table values (?,?) where there are 2 rows of data but second one is invalid returns: undef (correct) ArrayTupleStatus = (-1, [1772, 'ORA-01722...']) (incorrect as this should be (1, [1772, 'ORA-01722...']) probably same issue as 1). 3. update table where the update does not affect any rows returns: 0 (incorrect - should be 1 for 1 tuple executed) ArrayTupleStatus = (-1) (incorrect - should be (0) for no rows affected) If my conclusions look right to you I'll report them on rt. As an aside, I am finding execute_array returning the number of tuples executed rather than the number of rows affected a bit of a pain. It seems to make it easy to ascertain how many tuples were executed but hard to find out how many rows were affected when I would have thought the former is less useful and easily calculated as [EMAIL PROTECTED]). What I normally do on a single insert/update execute is: $affected = $sth->execute(@params) ERROR if ($
execute_array/execute_for_fetch (was: ANNOUNCE: Advanced DBI Tutorial Slides)
On Mon, Sep 04, 2006 at 05:21:17PM +0100, Martin J. Evans wrote: > > It did raise an issue where I could do with some clarification. The slides > say: > > Execute a statement for multiple values >$sth = $dbh->prepare("insert into table (foo,bar) values (?,?)"); >$tuples = $sth->execute_array(\%attr, [EMAIL PROTECTED], [EMAIL > PROTECTED]); > > returns count of executions (even ones that failed) and not rows-affected. > > but the latest DBI pod says: > > The execute_array() method returns the number of tuples executed, > or "undef" if an error occured. Like execute(), a successful exe- > cute_array() always returns true regardless of the number of tuples > executed, even if it's zero. See the "ArrayTupleStatus" attribute > below for how to determine the execution status for each tuple. > > I read that latter as if undef is returned 1 or more of the executes failed > but > now I read in the slides they suggest I may get a true value when an error > occurs. My experience with execute_array and Oracle shows I get: > > return 0, when none resulted in a change but no errors > e.g. > drop table mytest > create table mytest (a int primary key, b char(20)) > insert into mytest values (1,'onetwothree') > update mytest set b = ? where a = ? > $sth->bind_param_array(2, [2]) > $sth->bind_param_array(1, ['two']) > $sth->execute_array( { ArrayTupleStatus => [EMAIL PROTECTED] } ) > > returns 0 > > No, error, just no update occurred. It does not return 1 as the slide > suggests and does seem to be the number of rows changed - none. > > returns undef when there is an error > e.g. > as per above example but the bind_param_array for p2 is invalid e.g. 'xxx' > > This seems to also contradict the slide but agrees with the DBI pod. > > lastly I get undef if there are multiple rows but the last one errors. > e.g. as above except the bind_param_arrays are: > > $sth->bind_param_array(2, [1, 'xxx']); > $sth->bind_param_array(1, ['one', 'two']); > > returns undef but executes the 1,'one'. > > I am now unclear as to what is correct. Can you clarify? The pod is right and both the slides and DBD::Oracle are wrong, though in different ways. The behaviour should match that of the default execute_for_fetch method (which execute_array calls) provided by the DBI - shown below. Thanks! I've fixed my copy of the slide to say "returns count of executions, not rows-affected, or undef if any failed". Tim. p.s. I'd be grateful if you could file a bug at rt.cpan.org for DBD::Oracle. sub execute_for_fetch { my ($sth, $fetch_tuple_sub, $tuple_status) = @_; # start with empty status array ($tuple_status) ? @$tuple_status = () : $tuple_status = []; my ($err_count, %errstr_cache); while ( my $tuple = &$fetch_tuple_sub() ) { if ( my $rc = $sth->execute(@$tuple) ) { push @$tuple_status, $rc; } else { $err_count++; my $err = $sth->err; push @$tuple_status, [ $err, $errstr_cache{$err} ||= $sth->errstr, $sth->state ]; } } my $tuples = @$tuple_status; return $sth->set_err(1, "executing $tuples generated $err_count errors") if $err_count; return scalar(@$tuple_status) || "0E0"; }