these files come from a well known distributor so there is little to no concern of anything malicious

I've read stories about manufactured floppies and/or CDs with viruses. I've read stories about manufactured audio CDs with root kits. I'm not talking about random stuff you pull down from the seedy side of the internet, either. I'm talking about big-name manufacturers.

The company may not be malicious, but the guy who produces this data may be having a bad day, found a new job, and is going to quit tomorrow.

In my experience, it's actually easier to use placeholders than not. On top of that, it's more secure, and may even be faster at runtime (for example, there's less for the DB server to parse and mangle, plus if you prepare a single query and use it multiple times, there's a bunch of text that only needs parsing once). My experience says that there is no downside to using placeholders. Arguably there is a bit more code to deal with (especially if you're manually binding variables), but there doesn't have to be, and if you try to compare the amount of code for an insecure (not-working in your case) snippet vs a secure (working) snippet, that's apples and bananas anyway.

I also read about the danger of inserting literal values in SQL but these files come from a well known distributor so there is little to no concern of anything malicious and I would

As you've clearly demonstrated, malicious intent is not required to break improper creation of SQL string literals. You say it's of little concern, yet it's preventing your code from working.

prefer to make the least amount of changes to this script as possible at the moment.

How can it get any simpler than moving a variable to the next line and adding a question mark?

$dbh->do(
   'INSERT INTO Foo (
              bar,
              baz
           ) VALUES (
              ?, ?
           )
   ',
   undef,
   $bar, $baz
);
[download]

How can it get any simpler than moving a variable to the next line and adding a question mark?

Is this your way of saying that I can in fact use a placeholder for that specific value and it will solve my problem? I just read about undef today and wasn't sure if it fit my situation or not.

At some point I will have to go through all the code and see about making some more corrections to improve the overall performance/security but I just need this part working asap.

Thanks for the input.

Is this your way of saying that I can in fact use a placeholder for that specific value and it will solve my problem?

Yes. I thought you knew that.

I just read about undef today and wasn't sure if it fit my situation or not.

It sounds like you didn't read the docs for do.

The list of values to be bound starts at the third parameter. If one wants to pass values to be bound, one needs to specifies a value for do's second parameter. The undef is the default value for that parameter.

There is another, I think very compelling, reason to use placeholders:   it enables you to prepare your query only once, and then re-use it (as you say...) “several hundred thousand times.”   The difference in efficiency can be quite compelling.

You are executing one query, and the query does not change.   Only the exact values being inserted each time.   Therefore, the database engine only needs to parse the SQL once, build one execution-plan, and then just keep re-using it over and over.   The data that corresponds to the various placeholders is never considered to be “part of the SQL string,” because of course (when you do it this way...) it isn’t.

You should also read-up on transactions.   You probably want to wrap this insertion-loop into a transaction, and COMMIT that transaction (and start a new one) every few thousand records or so (and at the end).

Before you go too much farther, Google® this:   bulk data inserts.

Another, less elegant IMHO, option would be to use $query = sprintf "insert into da_table values (%s)", $dbh->quote("kjasdfkj'asdflkjasef'sadfhasdfsadf'asdfhasfd'");

Sure, but it's slower and more complicated than placeholders, and easier to get wrong. Why even suggest it? Is there a case where you can't use placeholders?

---

Improve your skills with Modern Perl: the free book.

Actually I did run into a Sybase bug a few years ago where the placeholders resulted in the connection being dropped.

Jason L. Froebe

Blog, Tech Blog

Sybase can only have one active statement handle, but will transparently clone a database handle when necessary (unless you set the "I don't want that" flag).

This means you may run into issues like deadlocks if you're, e.g., inserting in the same loop that you're selecting and fetching in (or other strangeness from having two separate sessions).

And then I also ran into a bug where the cloned database handle didn't have the same client character set as the original (reported on the DBI mailing list).

Update: And I just noticed that the OP doesn't mention Sybase anywhere...so this point may be moot anyway...

I remember being unable to do that for a numeric field. Do I remember incorrectly?

As a side note, this SQL command gets called several hundred thousand times when run.

At that quantity, it is probably worthwhile to turn your insert into a print, print to a file, and use a bulk loader (or print to a pipe of the bulk loader can handle that). You don't mention what database it is, so I don't know what's available.

And if you stick with inserts, consider using RaiseError, which saves you from checking every DBI call, and you can wrap entire blocks in eval if you need to trap errors and handle them specially.