There are a few places that PostgreSQL chokes in checksetup code, before it can even get to the bz_ functions in Bugzilla::DB. These need to be fixed before I can fix each of those individual bz_ functions to be cross-database compatible.

These are simple fixes. I've verified that they work on Pg and on MySQL.

Comment on attachment 176083 [details] [diff] [review] Fix bad quoting and insertion of "NULL" as a string >-$sth = $dbh->prepare("SELECT description FROM products"); >+my $sth = $dbh->prepare("SELECT description FROM products"); this causes "my" variable $sth masks earlier declaration in same scope at checksetup.pl line 2386. otherwise looks good.

(In reply to comment #2) > this causes > "my" variable $sth masks earlier declaration in same scope at checksetup.pl > line 2386. Oh, that's part of the patch from bug 284172. :-) Oops. :-)

Comment on attachment 176083 [details] [diff] [review] Fix bad quoting and insertion of "NULL" as a string >Index: checksetup.pl >@@ -2230,23 +1651,21 @@ > sub AddFDef ($$$) { > my ($name, $description, $mailhead) = (@_); > >- $name = $dbh->quote($name); >- $description = $dbh->quote($description); Are you sure it's safe to leave this out? Can't the function be ever called with user-supplied or otherwise unsafe data? (That's not suggestion, that's really just question :-) ). >@@ -2384,20 +1804,26 @@ > ########################################################################### > # Create initial test product if there are no products present. > ########################################################################### >-$sth = $dbh->prepare("SELECT description FROM products"); >+my $sth = $dbh->prepare("SELECT description FROM products"); I remember this change was already in another patch :-). > $dbh->do("INSERT INTO fielddefs " . ... >+ $dbh->do(q{INSERT INTO products(name, description, milestoneurl, ... >+ $dbh->do(qq{INSERT INTO versions (value, product_id) Can't we somehow make all the quoting more consistent? We are using single quotes, double quotes, q{}, qq{} all over the code... But thats probably more question for some general rules than for this patch... Otherwise, it looks good...

> Are you sure it's safe to leave this out? Can't the function be ever called > with user-supplied or otherwise unsafe data? (That's not suggestion, that's > really just question :-) ). it's save; using sql placeholders automatically performs the quoting.

OK, this one's all better. :-)

Comment on attachment 176226 [details] [diff] [review] v2 r=glob

Checking in checksetup.pl; /cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl new revision: 1.359; previous revision: 1.358 done