1

I'm doing some cleanup and transformation on data (that part is done, whew), and need to insert it into a MySQL table. Having done this kind of thing in Perl previously, I assumed that, as part of processing, it would make sense for me to structure the data as an associative array with the keys being the same as the field names I need to load them into - that way, it would be easy to construct a prepared statement simply by looping over the keys and producing a list of both the named placeholders and the matching values.

However, I can't seem to make this work in PHP/PDO. Test code:

$x = <<<EOD
1 1 1 1 1
2 2 2 2 2
3 3 3 3 3
4 4 4 4 4
EOD;

$fields = array('name', 'job', 'wallet_size', 'inseam', 'pet_name');
foreach(explode("\n", $x) as $line){
    $data = array_combine($fields, explode(' ', $line));

    # print_r($data);

    $stmt = $dbh->prepare('INSERT INTO foobar VALUES('.':'.implode(', :', $fields));

    foreach($fields as $field){
        $stmt->bindParam(':'.$field, $data[$field]);
    }
    $stmt->execute();
}

Frankly, it feels too... graceless and hacky to work - and it doesn't.

Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1' in tst.php:24

There is a correct way to do this, right? I'd appreciate it if someone would introduce me to the appropriate PHP-flavored phrasing for it.

  • 3
    did you try to print `'INSERT INTO foobar VALUES('.':'.implode(', :', $fields)`? Looks like you just missing closing ")" `'INSERT INTO foobar VALUES(' . ':' . implode(', :', $fields) . ')'` – E_p Oct 07 '16 at 21:55
  • @E_p - well done! I *did* echo it - the 'inside' part, not the whole thing - and it looked OK. You're right, it was the closing paren. – bluewater_sailor Oct 07 '16 at 22:32

1 Answers1

4

It's hard to debug SQL when you're staring at the PHP code that formats SQL, instead of the final SQL string itself. I suggest you always create a string variable so you can output it during debugging.

$sql = 'INSERT INTO foobar VALUES('.':'.implode(', :', $fields);
echo "$sql\n";
$stmt = $dbh->prepare($sql);

Outputs:

INSERT INTO foobar VALUES(:name, :job, :wallet_size, :inseam, :pet_name

Now it's very easy to see that you forgot the closing ) at the end of that INSERT statement!

Also, your use of PDO is more difficult than it needs to be. You don't need to use named parameters. You don't need to use bindParam(). Here's how I would write this code:

$fields = array('name', 'job', 'wallet_size', 'inseam', 'pet_name');

$columns = implode(',', $fields);
$placeholders = implode(',', array_fill(1, count($fields), '?'));
$sql = "INSERT INTO foobar ($columns) VALUES ($placeholders)";
echo "$sql\n"; // use this during debugging
$stmt = $dbh->prepare($sql);

foreach(explode("\n", $x) as $line){
    $param_values = explode(' ', $line);
    $stmt->execute($param_values);
}

Tips:

  • Prepare the query once, and re-use the prepared statement for each row of data you want to insert.
  • Pass an array of data values as an argument to execute(). This is easier than using bindParam().
  • Use positional parameter placeholders (?) instead of named parameter placeholders when your data is in a simple array instead of an associative array.
Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
  • 1
    Beautiful - just what I was looking for. Thank you, Bill! @E_p has spotted the fact that I'd missed the paren, but your answer included that as well as something that makes sense when you read it (which my code didn't, to my eye.) Much appreciated! – bluewater_sailor Oct 07 '16 at 22:36
  • When you expand on somebodies answer for perfection(without credit) make sure it is perfect. `$sql = "INSERT INTO foobar ($columns) VALUES ($placeholders)";` should be `$sql = sprintf("INSERT INTO foobar (%s) VALUES (%s)", $columns, $placeholders);` – E_p Oct 07 '16 at 22:44
  • I'll agree with the rewrite Bill. However, using named placeholders are a lot easier to track than `?`'s. I use both MySQLi_ and PDO prepared statements, yet use named placeholders 99% of the time in PDO; but that's just me ;-) – Funk Forty Niner Oct 07 '16 at 23:23
  • @E_p, in this case, it's unnecessary to use sprintf, and using sprintf would make the code less clear. – Bill Karwin Oct 07 '16 at 23:33
  • 1
    @Fred-ii-, sure, that's a matter of personal style. It's a tradeoff. Using positional parameters made it easy to use array_fill() to create the list of placeholders to match the number of fields. – Bill Karwin Oct 07 '16 at 23:36
  • In any case you still a person who takes credits for other people answers ;) – E_p Oct 07 '16 at 23:49