Re: Re:{2} Leashing DBI

[Masem]

The only problem that I have with this specific method is that you no longer use placeholders, which means you need to take appropriate care to quote and/or detaint all input into the SQL statements.

Now, let me offer to take this one step further, and cleaner: stick all SQL statements into values of a hash in a separate module:

package SQL_Helpers;

my %sql_statements = ( 
    SELECT_USER_INFO => q/SELECT userid, username, password FROM user_
+table WHERE userid=?/,
    UPDATE_USER_INFO => q/UPDATE user_table set password=? WHERE useri
+d=?/,
    ...yada yada yada...
    );

sub sql {
    my ( $dbh, $sql, @data ) = @_;
    return undef if !exists( $sql_statements{ $sql } );
    my $sth = $dbh->prepare( $sql_statements{ $sql } )
        or die DBI::errstr;
    $sth->execute( @data ) or die DBI::errstr;
    return $sth;
}

...

package MyPackage;
use SQL_Helpers qw/:all/;

my $sth = sql( $dbh, UPDATE_USER_INFO, $password, $userid );

I would even go a step farther, possibly being able to class-ify this so that one can make methods that are created at runtime that correspond to the various SQL statements:

my $sth = $sql_helper->UPDATE_USER_INFO( $password, $userid );

As well as incorporating prepare_cached details into this class such that things can be speeded up in the SQL side. Notice that there are no dies in the main block of code, and to me this is even more readable that typical cases. Now, these ideas seem rather simple that I would figure there's already a CPAN module for this, but I thought the same of both my Game::Life and Tie::Hash::Stack too...Maybe I'll take a look into developing such a class if nothing currently exists.

The only major problem with this is that sometimes the SQL statement and the code that calls it are closely nit. For example, one might try to do a SELECT listing all specific fields (more than 5, say), and using fetchrow_array with a long my() statement to collect them. If one then wanted to add another field, both the SQL statement and the perl code that called it would have to be changed at the same time; too much of a separation between the SQL statement and perl code could be confusing. But another option is to have pseudo variables in the SQL statement as well, and pass parameters by a hash; the class would be able to determine what order of parameters to pass based on placement of the placeholders in teh SQL statements.

Example SQL: UPDATE user_table SET password=?password WHERE userid=?us
+erid
Example call to sql:
my $sth = $sql_helper->UPDATE_USER_DATA( 
             { userid=>$userid, 
             password=>$password } 
          );

The only problem going this far is you are now losing some speed aspects for 'beauty' of code, which is always a plausable tradeoff. I might simply try this to see how bad the time difference is, though I suspect most of it is buried in the SQL server details.

---

Dr. Michael K. Neylon - [e-mail] || "You've left the lens cap of your mind on again, Pinky" - The Brain

[pope]

prepare_cached() (and other similar tricks of reusing the same statement handle to avoid reparsing the same SQL statement) doesn't give any performance benefit if the underlying database doesn't support that, i.e. MySQL.

For DBD::mysql, when we call $sth->execute, the driver will eventually call mysql_real_query() which reparses the same SQL statement. Placeholders are emulated by the driver.
The following is a simple test which compares the performance between using of reusable $sth and using the "unrecommended" way. With DBD::mysql, of course:

my $res = $dbh->selectall_arrayref(<<'SQL');
 SELECT id, title FROM tbl_articles
 SQL

 my @subs = (
    [
        'Using string subst' => sub {
            for (@$res) {
                $dbh->do(
                'UPDATE tbl_articles SET title = '.
                $dbh->quote('##'.$_->[1])." WHERE id = $_->[0]");
            }
        },
    ],
    [
        'Using placeholders' => sub {
            my $sth = $dbh->prepare(<<'SQL');
 UPDATE tbl_articles SET title = ? WHERE id = ?
 SQL
            for (@$res) {
                $sth->execute(@{$_}[1,0]);
            }
        },
    ],
 );

 for (@subs) {
    print "$_->[0]:\n";
    timethis(100, $_->[1]);
 }

And my result:

 Using string subst:
 timethis 100: 15 wallclock secs ( 2.33 usr +  0.78 sys =  3.11 CPU)
 Using placeholders:
 timethis 100: 15 wallclock secs ( 2.45 usr +  0.97 sys =  3.42 CPU)

So if we stuck to MySQL, we needn't worry about placholders, prepare_cached() or something like that. No performance penalty for not harnessing them.

I agree with you about sacrificing performance for 'beauty' of code. This even worse for wrapper modules (around DBI) which try to add more database independency than that provided by DBI. Take a look of how DBIx::Recordset solves the problem of alleviating different ways of each DBMS of doing partial select. Really unsatisfactory from performance standpoint.

[Masem]

First, it's not that placeholders provide a performance boost (I'd expect a decline, actually, but...).. but they *are* important in the detainting and security of your database. Even if your server doesn't support placeholders, this is still the sanest thing to use when you can, since if the server doesn't have placeholders, the functionality is replicated by the appropriate DBI glue. This also applies to prepare_cached(); if the database server doesn't support it, it will do it's best to handle it in any case; in cases where it doesn't, it simply will pass prepare_cached() to prepare().

But in the situation I was talking about, I was more thinking about the penalty of creating a hash to pass, parsing the SQL, accessing the hash and calling prepare & execute all in a separate area, as opposed to calling prepare & execute without extra baggage. AGain, I doubt it's much, and the small hit may be more than outweighed by the ease of programming in such a fashion.

---

Dr. Michael K. Neylon - [e-mail] || "You've left the lens cap of your mind on again, Pinky" - The Brain