Using DBI methods
bbaetz at student.usyd.edu.au
Sun Aug 18 13:49:37 UTC 2002
This got kind of long, but do read it all carefully.
In bug 163290, I'm moving the various DB stuff into a separate file,
For the moment, this is being transferred fairly literally into an OO
However, it seems to me that it may be worth being able to use the DBI
methods directly. |perldoc DBI| lists those, for the interested.
1) Can use stuff like:
my ($a, $b) = $dbh->selectrow_array("SELECT a,b FROM foo WHERE
or selectall_arrayref, and so on
which is nicer/cleaner/faster/easier to read than our sendsql+fetch stuff
2) We get to use DBI's error handling + exceptions - currently we only
check for errors in one place, which is bad.
Note that all of these can reasonably easily be added to the current
setup, but it starts to become less worthwhile/more complicated/etc as we
3) Can be faster - by using the routines as written in C in the DBI/DBD
- unlikely to be really noticable since we don't tend to move large
volumes of data to/from the db, but every bit helps.
4) No more state stack
- don't have to worry about forgetting to push/pop the state stack -
simply get a new statement handle when you need one from teh globally
available dbh when needed.
1) New syntax required
- We can (and will) keep the old syntax arround for backwards
compatability, though, for the reasonably future.
- Mixing FetchOneColumn() and fetchrow_arrayref (for example) on ehte
same statement handle won't work, because of the internal buffer
MoreSQLData needs to determine emptyness.
2) Its slightly messier
- Need to keep statment objects arround explicitly, rather than
3) We need to up the required DBI version
- The currently required version dates to July 1999 (1.13). To use the
HandleError stuff to do custom SQL error messages, we need 1.21 (Feb 2002)
- Its also needed to subclass DBI, which is what we need for taint
checking, and supporting the old (current) syntax (speaking of which, I
should email the dbi list about the taint stuff...)
- It has a lot of the cross-db stuff for transactions which we will need
at some point
- DBI-1.21 comes with debian stable and RH7.2/7.3 at least. Its not that
hard to install it though, anyway
- If DBI does add a 'TaintInOnly' mode, then we'll require that at some
point, although probably not right away (or even at all for 2.18)
4) we need to up the required perl version to 5.005_03
- that DBI version requires 5.005_03. We've been talking about it for a
while, and 5.8 is now out, so requring the latest 5.005 release shouldn't
be an issue, although I know that afranke will disagree...
- Do note that the latest DBI version has a note that it may not support
5.005 for much longer. This may become an issue for us at some point in
What do people think about the above?
I also plan to remove the following two features:
a) SQL log should indent the log as statements are pushed to the stack
- If we no longer have a stack and have sth's which can be finshed in
an order different to when they were started, this is a pain to do.
- The results don't really look any better, because very very very few of
our SQL statements fit into 80 columns, let alone 80-8*depth. so it
- More importantly, this doesn't actually work. It took me 15 minutes to
work out why not, and it hasn't worked since it was checked in almost
two years ago. Why? Because the case in |SavedSQLStates| is wrong.
Why didn't this trigger a 'used only once' warning? Because it got
added to the silliness stuff in a warning fix-up (bug 72862) :)
b) In theory, Param("shutdownhtml") will currently work even for scripts
which don't |require 'CGI.pl'|.
Pre-templatisation/2.16, the check was in PutHeader, which was too
late (and not called for non-CGIs anyway) Now, its in CGI.pl's toplevel
The current behaviour of having MoreSQLData() return 0 is a pain to
re-implement, plus its buggy, and may cause stuff to happen which
shouldn't (eg what if one of these calls is trying to regenerate the
versioncache, or its a security check, or something?)
(I just checked, and regenerating the versioncache with shutdownhtml set
does result in one which is empty.)
Eventually this will happen somewhere apart from CGI.pl, but for the
moment... This will cause different behaviour in non-cgi stuff, but I
consider an error (which Bugzilla::DB::connect will generate) to be
preferable than generating bogus data/empty versioncache/etc
I'd be incredibly surprised if any of those programs work with shutdown
html anyway - collectstats, for example, dies when it can't read from the
product table a product which is in the versioncache.
(I'm tossing up between |connect| returning undef, dieing, or throwing an
exception, and leaning towards the exception, which will basically mean a
If you object to the removal of any of the above please let me know ASAP.
More information about the developers