[Koha-patches] [PATCH] Squash warnings by checking if $type is defined before comparing it.

[Joe Atzberger]

I think you are missing the main thrust of the DBI
perldoc<http://search.cpan.org/%7Etimb/DBI-1.607/DBI.pm#finish>.
Our $sth is not hanging around with unfetched data, since there are only two
possiblilities:

   1. Successful on first try, return ($data).  $sth immediately falls out
   of scope and is flushed the recommended way, i.e. automatically.
   2. Unsuccessful on first try, $sth reassigned.  Since $sth was the only
   reference to that first handle, it is also flushed automatically.

If the second $sth were distinct, there is the possibility that finish might
have been intentional and beneficial, but as it is, there is zero benefit to
retaining these finish statements.   They represent a mistaken common
practice in previous iterations of Koha.

I think this is the same odd function we were discussing with Paul and I
agree there is the obvious tension between the queries (espeicially a LIKE
query on firstname!) and the fact that we only return one row in any case.
If that's the intent we should probably LIMIT 1 from the outset.

--Joe

On Tue, Nov 4, 2008 at 4:05 PM, Galen Charlton
<galen.charlton at liblime.com>wrote:

> Hi,
>
> This patch does more than what its description claims.  In this case,
> the calls to $sth->finish() *are* technically legitimate, since a
> search on userid or firstname can produce more than one row in the
> result set.  Please resubmit, either backing out of the $sth->finish
> change or (as the DBI documentation suggests), replace the use of
> finish() by with selectrow_*.
>
> Regards,
>
> Galen
>
> On Thu, Oct 30, 2008 at 12:10 AM, Joe Atzberger
> <joe.atzberger at liblime.com> wrote:
> > ---
> >  C4/Members.pm |    6 ++----
> >  1 files changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/C4/Members.pm b/C4/Members.pm
> > index e756ffb..7ee9eb3 100644
> > --- a/C4/Members.pm
> > +++ b/C4/Members.pm
> > @@ -527,7 +527,7 @@ SELECT borrowers.*, categories.category_type,
> categories.description
> >  FROM borrowers
> >  LEFT JOIN categories on borrowers.categorycode=categories.categorycode
> >  ";
> > -    if ( defined $type && ( $type eq 'cardnumber' || $type eq
> 'firstname'|| $type eq 'userid'|| $type eq 'borrowernumber' ) ){
> > +    if (defined($type) and ( $type eq 'cardnumber' || $type eq
> 'firstname'|| $type eq 'userid'|| $type eq 'borrowernumber' ) ){
> >         $information = uc $information;
> >         $sth = $dbh->prepare("$select WHERE $type=?");
> >     } else {
> > @@ -535,14 +535,12 @@ LEFT JOIN categories on
> borrowers.categorycode=categories.categorycode
> >     }
> >     $sth->execute($information);
> >     my $data = $sth->fetchrow_hashref;
> > -    $sth->finish;
> >     ($data) and return ($data);
> >
> > -    if ($type eq 'cardnumber' || $type eq 'firstname') {    # otherwise,
> try with firstname
> > +    if (defined($type) and ($type eq 'cardnumber' || $type eq
> 'firstname')) {    # otherwise, try with firstname
> >         $sth = $dbh->prepare("$select WHERE firstname like ?");
> >         $sth->execute($information);
> >         $data = $sth->fetchrow_hashref;
> > -        $sth->finish;
> >         ($data) and return ($data);
> >     }
> >     return undef;
> > --
> > 1.5.5.GIT
> >
> > _______________________________________________
> > Koha-patches mailing list
> > Koha-patches at lists.koha.org
> > http://lists.koha.org/mailman/listinfo/koha-patches
> >
>
>
>
> --
> Galen Charlton
> VP, Research & Development, LibLime
> galen.charlton at liblime.com
> p: 1-888-564-2457 x709
> skype: gmcharlt
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/koha-patches/attachments/20081106/d02be9e5/attachment-0002.htm>