I wrote a simple Postgres database application over the last few days
and elected to use the DBD:PgPP module in conjuction with
DBIx::SQLEngine as they are both included in the ActivePerl distribution
and I like the general philosophy. I had not used DBD::PgPP before. It
turns out that there are a few rather basic bugs. Since noone seems to
currently be maintaining this module, perhaps ActiveState could apply
fixes in their distribution?
In increasing level of complexity, here are the issues I found (all in
.../DBD/PgPP.pm):
1. Simple typo at line 343
my $params = $sth->FETCH('pgpp_param');
should be
my $params = $sth->FETCH('pgpp_params');
2. Simple typo at line 682
$self->{finisy} = undef;
should be
$self->{finish} = undef;
3. The logic for substituting parameters into SQL queries near
line 360 is flawed. If the values being substituted include
question marks, then the substitution gets out of sync. I
elected to fix this by combining all the parameters into a
single string and substituting in a single step. I changed
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 replaced it with
my @workparms = ();
for (my $i = 0; $i < $num_param; $i++) {
my $dbh = $sth->{Database};
push @workparms, $dbh->quote($params->[$i]);
}
$statement =~ s/\?(?:\, \?)*/join(',',@workparms)/e if $num_param
> 0;
4. Under certain conditions (not fully investigated) the
{'row_description'} hash in DBD::PgPP::Protocol, referenced
in the code using $pgsql, ends up containing residual data
from a previous query rather than valid information from the
current query. This caused me trouble when a previously used
cached query was being reused. PgPP was trying to overwrite
existing (valid) information in the statement handler with
invalid information. I simply do not have the time right now
to research a correct fix. The pragmatic solution I decided
on was to change line 381 from
if ($pgsql->{row_description}) {
to
if ($pgsql->{row_description} and not
$sth->FETCH('NUM_OF_FIELDS')) {
This (probably partial) fix is probably valid in that we are in the
middle of executing the query and would scarcely be changing the
rows we are handling in the middle of the query. If the existing
information is incorrect, we are probably in big trouble anyway.
Here is a summary of the changes in the form of a diff:
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
> $statement =~ s/\?(?:\, \?)*/join(',',@workparms)/e if $num_param > 0;
381c384,389
< 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')) {
682c690,691
< $self->{finisy} = undef;
---
> # $self->{finisy} = undef;
> $self->{finish} = undef;
If anyone has additional fixes for this module, perhaps they would be so
kind as to let me know. Obviously, if someone is either more
knowledgable in this area (or has lots of time) improved fixes are very
welcome.
--
Tim Bolshaw
[EMAIL PROTECTED]
_______________________________________________
ActivePerl mailing list
[email protected]
To unsubscribe: http://listserv.ActiveState.com/mailman/mysubs
