[Koha-devel] Question on proper/preferred DBI usage?

[Alan Millar]

I definitely do not want to get into a flame war on this, so if
it turns that direction, I'll drop the subject.

On Thu, May 16, 2002 at 03:42:30PM +1200, Gynn Lomax wrote:
> The reason it has been done this way is simple:
> I like to be able to see what values I am assign to where.
 
> With the values embedded into the query it means I can directly see what
> they match up with, e.g.:
> 
> $query = "Insert into biblioitem set
> biblionumber = $biblioitem->{'biblionumber'},
> itemtype         = $biblioitem->{'itemtype'}";

I haven't looked at enough of the code, but I haven't see 
much of that in the code.  I see things like:

        $sth=$dbh->prepare("insert into biblioitems (biblioitemnumber, biblionum
ber, volume, number, itemtype, isbn, issn, dewey, subclass, publicationyear, pub
lishercode, volumedate, volumeddesc, illus, pages, notes, size, place, lccn, mar
c) values ($biblioitemnumber, $biblionumber, $q_volume, $q_number, $q_itemtype,
$q_isbn, $q_issn, $q_dewey, $q_subclass, $q_publicationyear, $q_publishercode, $
q_volumedate, $q_volumeddesc, $q_illus, $q_pages, $q_notes, $q_size, $q_place, $
q_lccn, $q_marc)");

and I'm definitely having a hard time matching up which variable goes
with which column name.
 
> With the bind values method you get:
> $query = "Insert into biblioitem set
> biblionumber = ?,
> itemtype         = ?";
> 
> followed by later:
> $sth->execute($biblioitem->{'biblionumber'}, $biblioitem->{'itemtype'});
> 
> While for this simple example it is easy to see which value matches up, for
> more complicated queries, and those that have a lot of values it can start
> to get mor confusing.

With this method, as well as the example above (straight from marcimport.pl),
in my opinion the main problem is formatting.

Compare, for example, the following actual code:

-------------------------------------------------------------

        my $q_isbn=$dbh->quote($isbn);
        my $q_issn=$dbh->quote($issn);
        my $q_lccn=$dbh->quote($lccn);
        my $q_volume=$dbh->quote($input->param('volume'));
        my $q_number=$dbh->quote($input->param('number'));
        my $q_itemtype=$dbh->quote($input->param('itemtype'));
        my $q_dewey=$dbh->quote($input->param('dewey'));
        my $q_subclass=$dbh->quote($input->param('subclass'));
        my $q_publicationyear=$dbh->quote($input->param('publicationyear'));
        my $q_publishercode=$dbh->quote($input->param('publishercode'));
        my $q_volumedate=$dbh->quote($input->param('volumedate'));
        my $q_volumeddesc=$dbh->quote($input->param('volumeddesc'));
        my $q_illus=$dbh->quote($input->param('illustrator'));
        my $q_pages=$dbh->quote($input->param('pages'));
        my $q_notes=$dbh->quote($input->param('note'));
        my $q_size=$dbh->quote($input->param('size'));
        my $q_place=$dbh->quote($input->param('place'));
        my $q_marc=$dbh->quote($input->param('marc'));

        $sth=$dbh->prepare("insert into biblioitems (biblioitemnumber, biblionum
ber, volume, number, itemtype, isbn, issn, dewey, subclass, publicationyear, pub
lishercode, volumedate, volumeddesc, illus, pages, notes, size, place, lccn, mar
c) values ($biblioitemnumber, $biblionumber, $q_volume, $q_number, $q_itemtype,
$q_isbn, $q_issn, $q_dewey, $q_subclass, $q_publicationyear, $q_publishercode, $
q_volumedate, $q_volumeddesc, $q_illus, $q_pages, $q_notes, $q_size, $q_place, $
q_lccn, $q_marc)");

-------------------------------------------------------------

with this functional equivalent:

-------------------------------------------------------------

        $sth=$dbh->prepare("insert into biblioitems 
	(biblioitemnumber, biblionumber,
	 volume, number, 
	 itemtype, isbn, issn,
	 dewey, subclass, 
	 publicationyear, publishercode,
	 volumedate, volumeddesc, 
	 illus, pages, notes,
	 size, place,
	 lccn, marc)
	values (?, ?, ?, ?, 
	 ?, ?, ?,
	 ?, ?, ?, ?,
	 ?, ?, 
	 ?, ?, ?,
	 ?, ?, ?, ?)   ");
        $sth=$dbh->execute(
	 $biblioitemnumber, $biblionumber,
	 $input->param{'volume'}, $input->param{'number'},
	 $input->param{'itemtype'},$input->param{'isbn'},$input->param{'issn'},
	 $input->param{'dewey'}, $input->param{'subclass'},
	 $input->param{'publicationyear'}, $input->param{'publishercode'},
	 $input->param{'volumedate'}, $input->param{'volumeddesc'},
	 $input->param{'illustrator'}, $input->param{'pages'}, $input->param{'note'},
	 $input->param{'size'}, $input->param{'place'},
	 $input->param{'lccn'}, $input->param{'marc)
	);

-------------------------------------------------------------

I'm sure it is just a matter of what each person is accustomed to,
but I can match up parameters easier in the second example than
I can in the original.

> For the very few queries I have run across that are executed more than once,
> the values being put into them are an unknown list (in terms of length),
> e.g.
> 
> $query = "Insert into deleteditems values ("
> foreach my $value (@results) {
>     $value  = $dbh->quote($value);
>     $query .= "$value,";
> } # foreach
> 
> and I'm unsure if the bind values method can handle this, so avoided it.

This code snippet looks like it is inserting an "unknown list" of 
column values into a table, on the assumption that the unknown 
list just happens to match up with the exact correct order of 
columns in the table.   This is different than the original issue
which is in the context of inserting multiple rows, not columns.

I'm definitely going to be stepping into hot water here, and so I must
apologize in advance for that.  But as to the columns, it seems to
me to be unsafe coding practice to insert values into a table 
without column names, because somebody somewhere is going to do 
some db modification, whether through custom changes or version 
upgrades, and some column will missing or out of order and it will
break.  Hashes perhaps are a good solution for this type of thing.

In any case, the bound parameter method works just fine for this:

 $query = "Insert into deleteditems values ("
 foreach my $dummy (@results) {
     $query .= "?,";
 } # foreach
 $query=~s/,$/\)/;
 $sth->execute(@results);

And there is probably a shorter way to do the correct number
of question marks via a count of @results.

> The other advantage of the method I have used is that you can print out the
> entire query before it is executed - so you can examine it for bugs. Whereas
> with the bind value method you never get to see the query in it's entirety.
 
OK, with the quoted method you only have to print one thing, while
with the bound method you have to print more.  That's true.  

I sure don't want to start a flame war, and if it sounds like I
am, please just ignore me and I'll drop it.  Thanks for the
education on another point of view.

- Alan


-- 

----
Alan Millar     --==> am12 at bolis.com <==--