PDO insert query, why isn't this working

Question

I stupidly built my web application with mysqli. Now, I'm trying to convert my data abstraction layer to pdo, but for some reason the insert query is giving me trouble. my shortcut insert function is called from the controller, and I was hoping to keep it in the name format with the table name and column/values array as the parameters.

I commented where I think the problem is below. Please help.

function insert($table, array $columns_values) {        

    // connect to db
    $dbh = $this->db_connect();

    $i = 0;

    $columns = array();
    $values  = array();
    $params  = array();

    foreach($columns_values as $column => $value) {

        $i++;

        $param = array($i => $value);
        array_push($params, $param);

        array_push($columns, $column);
        array_push($values, '?');

    }

    // turn arrays into comma separated list
    $columns =      implode(",", $columns);
    $values  =      implode(",", $values);


    $stmt = $dbh->prepare("INSERT INTO $table ($columns) VALUES ($values)");


    foreach ($params as $param_stmt) {

             // i think this is where the problem is
            foreach ($param_stmt as $placeholder => $value) {

                $stmt->bindParam($placeholder, $value);

            }


    }

    $stmt->execute();

    return $stmt;

} // end insert()

Answer 1

I wouldn't do it your way. After a few minutes, I came up with this:

/**
 * Function to insert a list of values to the database.
 * 
 * @param PDO    $pdo
 * @param string $table
 * @param array  $columns_values
 *
 * @throws \Exception
 * @throws \PDOException
 */
function insert_to_db(PDO $pdo, $table, array $columns_values) {
    //Some data validation.
    if (empty($columns_values)) {
        throw new \Exception("Insert at least one value.");
    }
    if (empty($table)) {
        throw new \Exception("Table may not be empty.");
    }

    //Implode all of column names. Will become the columns part of the query.
    $str_columns = implode(", ", array_keys($columns_values));

    //Implode all column names after adding a : at the beginning.
    //They will become the placeholders on the values part.
    $prepared_column_names = array_map(function ($el) {
        return ":$el";
    }, array_keys($columns_values));
    $prepared_str_columns  = implode(", ", $prepared_column_names);

    //The query itself. Will look like "INSERT INTO `$table` (col1, col2, col3) VALUES (:col1, :col2, :col3);"
    $query = "INSERT INTO `$table` ($str_columns) VALUES ($prepared_str_columns);";

    //Prepare the query
    $stmt = $pdo->prepare($query);

    //Iterate over the columns and values, and bind the value to the placeholder
    foreach ($columns_values as $column => $value) {
        $stmt->bindValue(":$column", $value);
    }

    //Execute the query
    $stmt->execute();

}

Things I changed

1. I don't instantiate the PDO object inside of the function. The function needs one in order to work, so it should be one of the arguments!
2. I throw Exceptions in case of an error. It's a better way of handling errors.
3. I use named placeholders instead of unnamed ones (:name vs ?). Produces more readable, easier to follow queries, should you ever need to debug.
4. Added comments to code. Again, you understand what you wrote now, but will you 6 months from now?
5. I made use of array_keys() to automatically generate an array full of keys (i.e. the columns), instead of looping and manually adding one.

Some tips

• When you instantiate a PDO object, make sure it throws PDOExceptions on error! Like so:

  new PDO($dsn, $user, $pass, array(PDO::PARAM_ERRMODE => PDO::ERRMODE_EXCEPTION));
  

  or

  $pdo = new PDO($dsn, $user, $pass);
  $pdo->setAttribute(PDO::PARAM_ERRMODE, PDO::ERRMODE_EXCEPTION);
  

  That way, you don't need to explicitly check for errors each time, you use a single try catch block for the whole thing, and you're good:

  try {
      insert_to_db($pdo, $table, $array_of_columns_and_values);
  }
  catch (\Exception $e) { //Will catch all kinds of exceptions, including PDOExceptions
      echo $e->getMessage();
  }
  

Answer 2

Without seeing what your original $columns_values array looks like.

Hope it helps

<?php 
function insert($table, $values){
    $dbh = $this->db_connect();

    $fieldnames = array_keys($values[0]);

    $sql = "INSERT INTO $table";
    /*** set the field names ***/
    $fields = '( ' . implode(' ,', $fieldnames) . ' )';
    /*** set the placeholders ***/
    $bound = '(:' . implode(', :', $fieldnames) . ' )';
    /*** put the query together ***/
    $sql .= $fields.' VALUES '.$bound;

    //INSERT INTO testtable( id ,col1 ,col2 ) VALUES (:id, :col1, :col2 )

    /*** prepare and execute ***/
    $query = $dbh->prepare($sql);
    foreach($values as $vals){
        $query->execute($vals);
        /*  Array
        (
        [id]   =
        [col1] = someval1
        [col2] = Someval21
        )*/
    }

}
//Multi Insert
$insert = array(array('id'=>'','col1'=>'someval1','col2'=>'Someval21'),
                array('id'=>'','col1'=>'someval2','col2'=>'Someval22'),
                array('id'=>'','col1'=>'someval3','col2'=>'Someval23'),
                array('id'=>'','col1'=>'someval4','col2'=>'Someval24')
);

insert('testtable',$insert);
?>

Answer 3

You haven't checked that your prepare() actually succeeded:

$sql = "INSERT ....";
$stmt = $dbh->prepare($sql);
if (!$stmt) {
    die($sql . $dbh->errorInfo());
}

Never assume a query succeeded, especially when you're building one totally dynamically as you are.