It seems to me that if something doesn't work, you'd check for errors.

What if you checked for errors (perhaps using RaiseError => 1)? What if you specify AutoCommit => 1? (AutoCommit should be on by default, though.)

ikegami is right, checking for errors helps. Shortcut:

In your statement

the tokens pickup, pmonth, pday, pyear are not symbols at the database level and thus have to be quoted. Use placeholders.

If the values you are inserting are in the column order, you dont need the column enumeration at all - you can just do

my $sql = "INSERT into teach_info values ($pickup,$pmonth, $pday, $pye
+ar)";
[download]

use DBI;
use strict;

my $dbh = DBI->connect(
    'DBI:mysql:gailbord_teachcoll',
    'gailbord_admin',
    'thotwp2',
    { RaiseError => 1 }
);
my $sth = $dbh->prepare("INSERT into teach_info values(?,?,?,?)");
$sth->execute($pickup, $pmonth, $pday, $pyear);
[download]

If the values you are inserting are in the column order, you dont need the column enumeration at all - you can just do

That is not a good idea because it makes the insert very fragile to possible database changes: if the field order changes your insert breaks. It is always wise to use explicit columns and then the values.

Regarding your recommendation to use placeholders (aka bind variables) you are absolutely correct. If your database supports it, DBI will take advantage and prepare the statements in the database, which can boost your transaction speed in great orders of magnitude depending on the underlying RDBMS.

The use of do() should be avoided at all times, and every statement should be prepared using bind variables. Of course There's More Than One Way To Do It, but in general terms, it makes a lot of sense to prepare your queries beforehand, much like you would in database procedure languages such a PL/pgSQL and Oracle's PL/SQL

Actually, the tokens '(pickup, pmonth, pday, pyear)' are field names for the following 'values ($pickup, $pmonth, $pday, $pyear)' part of the statement, which is perfectly valid, no quoting required. You can't assume that pickup, pmonth, pday and pyear are the only fields in the table either (others may be auto defaulted) or that the order of the fields in the table is the same table definition (they may have to be inserted in a different order).

The real problem is that the entire query is contained in single quotes and the variables will never be filled in (in the first example). Checking errors as has been suggested would confirm this. The query would never execute successfully with literals like $pickup in them. The second example may be equally broken depending on the values of the variables. Numbers can go unquoted, but string values would definitely need quotes around them. Again checking errors would confirm this to.

Maybe it's a simple case of table permission. The MySQL database user may be able to read the table but not insert values. Again with the checking errors.

I can't agree more on the use of place holders (++). This also works:

my $affInfo = $dbh->do(
 'INSERT into teach_info (pickup,pmonth,pday,pyear) values (?,?,?,?)',
 undef,
 $pickup, $pmonth, $pday, $pyear);
[download]

That said, placeholders do not necessarily have a place in once-off queries. What you absolutely MUST do when placing your data inline like this, is quote your data using the $dbh->quote() method. Make it a habit, even when you trust your variables, because you may reuse the code later.

Not only does ->quote() place quotation marks around your strings (failure to do so will make the query fail) but it will also escape potentially harmful characters that could be injected by a hacker.

my $name = "; DROP DATABASE mysql;";
my $sth = $dbh->prepare("INSERT INTO names (name) values ('$name')");
$sth->execute;
[download]

my $name = "; DROP DATABASE mysql;";
my $sth = $dbh->prepare("INSERT INTO names (name) values (".$dbh->quot
+e($name).")");
$sth->execute;
[download]

Google "SQL Injection".

FloydATC:

If you're going to use prepare then execute for a single-use query, I still advocate using placeholders. Here's why:

• The server has the opportunity^[3] to precompile the request in the form specified by the developer, rather than as-modified by the user^[1]. So maintenance programmers can easily review the SQL to see what the server will do, without having to trace the parameters to ensure that they're all properly quoted.
• It appears to be simpler^[2] to use placeholders than to properly apply the quote method to build SQL commands. Relying on the DBI module and associated DBD drivers to "do the right thing" when using placeholders is much simpler than ensuring you've properly quoted all values you're interpolating into an SQL string.

NOTES:

[1] Meaning that the SQL the server may precompile is known to the developer, rather than being modified by text manipulations. I realize that the resulting parse tree will be identical with proper use of quoting, #############

[2] Converting your example to use placeholders gives us:

my $name = "; DROP DATABASE mysql;";
my $sth = $dbh->prepare("INSERT INTO names (name) values (?)");
$sth->execute($name);
[download]

[3] If the driver supports it, then the server can compile the execution plan for the statement. Afterwards, no combination of quotes, semicolons, comments, etc. can make the server drop a table (or other action) not already in the execution plan.

...roboticus

I agree, $dbh->do() would have been much better as an example of a once-off query, and as a general rule placeholders produce code that is easier to read and debug. Where possible.

Otherwise, use $dbh->quote().

-- Time flies when you don't know what you're doing

I once searched for hours to find an error in my code, only to discover that one of the variables was empty and that its database field did not accept "undef", so the whole insert did not go through.