Do you see a problem with this?

There are lots of issues with your code. The first major one is that you have no error handling (as far I can see) and thus no idea what the problem is - checking the logs is always a good start. The next is that you are passing raw user input to your DB and are not using place holders to quote (automatically escape) your values. As your code stands if I passed it a param 'id' with a value like ' "; DROP TABLE users;.......' it would probably execute any SQL I wanted. Adding yourself as a root user is always a good trick when this happens (with remote access rights of course ;-)

There is loads of material on how to use databases well (safely and reliably) on this site. Start in Tutorials I am not familiar with the book you quote but if it taught you to generate code like you posted I'm afraid to say it is not much chop. This is a very good, short and free reference that covers things in more detail. A short guide to DBI by Dominus. I really suggest you read it. As a starter here is a useful template:

my $dbh = do_dbi_connect( $db_type, $db_name, $db_username, $db_passwo
+rd, @_ );

my $sql =<<SQL;
SELECT *
FROM some_table
WHERE
    some_val  = ? OR
    other_val = ?
SQL

# now exec our sql using bind values to get our sth
# these replace our ? placeholders and will be quoted correctly
my $sth = do_sql_sth( $dbh, $sql, $bind_val1, $bind_val2 );
while( my $res = $sth->fetchrow_array() ) {
    # blah
}
$sth->finish

##### STANDARD DATABASE FUNCTIONS #####

# do_dbi_connect( DB_TYPE, DB_NAME, DB_USERNAME, DB_PASSWORD, @OPTIONS
+ )
# connect to the specified DB_TYPE and DB_NAME using the DB_USERNAME a
+nd
# DB_PASSOWRD as well as passing any other @OPTIONS
# returns the DBH
sub do_dbi_connect {
    my ( $db_type, $db_name, $db_username, $db_password ) = ( shift, s
+hift, shift, shift );
    my $dbh = DBI->connect("dbi:$db_type:$db_name", $db_username, $db_
+password ) or die_nice( $DBI::errstr );
  return $dbh;
}

# do_sql( DBH, SQL, BIND_VAL1.....BIND_VALx )
# prepares, executes and closes sth in one call
# typical use for create tables, update tables
sub do_sql {
    my $dbh = shift;
    my $sql = shift;
    my $sth = get_sth( $dbh, $sql );
    $sth->execute(@_) or 
        die_nice( "Could not execute SQL statement\n\n$sql\n" . $sth->
+errstr() );
    $sth->finish;
}

# do_sql_sth( DBH, SQL, BIND_VAL1.....BIND_VALx )
# as for do_sql but returns the STH
# obviously does not call finish on the STH (so you have to remember)
# typical use for fetchrow_blah
sub do_sql_sth {
    my $dbh = shift;
    my $sql = shift;
    my $sth = get_sth( $dbh, $sql );
    $sth->execute(@_) or 
        die_nice( "Could not execute SQL statement\n\n$sql\n" . $sth->
+errstr() );
  return $sth;
}

# get_sth( DBH, SQL )
# returns a (cached) STH for the supplied DBH and SQL
sub get_sth {
    my ( $dbh, $sql ) = @_;
    my $sth = $dbh->prepare_cached($sql) or 
        die_nice( "Could not prepare SQL statement\n\n$sql\n" . $dbh->
+errstr() );
  return $sth;
}

# die nice( ERROR )
# dies with ERROR after printing a valid header and error message to b
+rowser
# you may choose to modify this to produce a BS message to the user li
+ke
# 'the server can not respond to your request at this time due to rout
+ine  maintenance'
# or some such rubbish. just make sure you get to see the ERROR!

sub die_nice { 
    print "Content-type: text/html\n\n$_[0]"; 
    die $_[0] 
}
[download]

cheers

tachyon

s&&rsenoyhcatreve&&&s&n.+t&"$'$`$\"$\&"&ee&&y&srve&&d&&print