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
