In a message dated 5/23/2005 12:46:11 P.M. Eastern Standard Time, [EMAIL PROTECTED] writes:
 
> Just after sending my last post on 18th May, I discovered that one of my
> fixes was totally wrong.  I apologise for that: never assume quick fixes
> made late at night will be correct!  I have been extremely busy and did
> not have a chance to post my revised fix until today.
>
> The revised post was to the logic for code around line 360, substituting
> parameters into SQL queries.  My original fix fails in basic cases.  My
> revised fix changes the original 0.05 code from
>
>        for (my $i = 0; $i < $num_param; $i++) {
>          my $dbh = $sth->{Database};
>          my $quoted_param = $dbh->quote($params->[$i]);
>          $statement =~ s/\?/$quoted_param/e;  
>        }
>
> and replaces it with
>
>        my @workparms = ();
>        for (my $i = 0; $i < $num_param; $i++) {
>          my $dbh = $sth->{Database};
>          push @workparms, $dbh->quote($params->[$i]);
>        }
>        my $ix = 0;
>        $statement =~ s/\?/$workparms[$ix++]/eg if $num_param > 0;
>
> I would be grateful if an RE wizard could verify this fix is correct.
> Assume that $statement can contain question marks at arbitrary
> locations.  Some parameters in @workparms may also contain question
> marks.  The objective is to replace each single question mark, in
> sequence, in the original $statement with the parameters in order.  Any
> question marks in the parameters should remain.  So, if the original
> $statement was "ab?cd?ef" and the paramaters were "1??2" and "3???4",
> $statement after substitution should be "ab1??2cd3???4ef".  Is the logic
> above OK?  It feels right, but my last quick fix to this was wrong.
don't know if i'm an ``re wizard'' or not, but here goes...  
the regex certainly does what you require (insofar as i understand it).  
interestingly, the /e switch is not needed because a scalar element of an array interpolates in the replacement string as expected for a scalar.   to evaluate the array element, the array index $ix must be evaluated, and to evaluate $ix the post-increment operator must be evaluated.   however, the /e switch will do no harm, and should produce exactly the same result in this instance except maybe for a small speed difference (positive? negative?).   the above is certainly true for activestate version 5.8, and i believe also for all activestate versions 5.x.  
 
hth -- bill walters  
 
>
> A diff of my version of PgPP.pm against the standard 0.05 version in
> ActivePerl 5.8.6 is now:
>
> 343c343
> <       my $params = $sth->FETCH('pgpp_param');
> ---
> >     my $params = $sth->FETCH('pgpp_params');
> 358a359
> >     my @workparms = ();
> 361,362c362,364
> <               my $quoted_param = $dbh->quote($params->[$i]);
> <               $statement =~ s/\?/$quoted_param/e;
> ---
> > #        my $quoted_param = $dbh->quote($params->[$i]);
> > #        $statement =~ s/?/$quoted_param/e;
> >         push @workparms, $dbh->quote($params->[$i]);
> 363a366,367
> >     my $ix = 0;
> >     $statement =~ s/\?/$workparms[$ix++]/eg if $num_param > 0;
> 381c385,390
> <               if ($pgsql->{row_description}) {
> ---
> >         # NOTE (by Tim Bolshaw): $pgsql->{row_description} often contains the
> >         #      row description from the previous query.  At least, this leads to
> >         #      problems when we are now executing a previously used cached query.
> >         # XXX What is the correct fix?
> > #        if ($pgsql->{row_description}) {
> >         if ($pgsql->{row_description} and not $sth->FETCH('NUM_OF_FIELDS')) {
> 682c691,692
> <       $self->{finisy}        = undef;
> ---
> > #    $self->{finisy}        = undef;
> >      $self->{finish}        = undef;
>
> Corrections, improvements and additional fixes to this module would be
> welcome.  I decided to stay with it (in conjunction with the excellent
> DBIx::SQLEngine) for a couple of small applications in spite of the
> rather worrying basic bugs.
> --
>   Tim Bolshaw
 
_______________________________________________
ActivePerl mailing list
[email protected]
To unsubscribe: http://listserv.ActiveState.com/mailman/mysubs

Reply via email to