creating a flexible update query with Php and pdo - problems with bindparam

Question

I'm updating my mysql functions to use PDO. I've got the hang of most of it but struggling with an update function to update multiple fields in a records. The function is in a class and I'm trying to keep it flexible to reuse with other tables etc.

Here's my function so far:

 public function dbUpdateRecord($table, $values, $where)
{


    $this->conn();
    $fieldNames = array_keys($values);
    var_dump($fieldNames);
    $set="";
    foreach ($fieldNames as $field) {
        $set .= " $field = :$field,";
    }

    //strip last comma
    $set = substr($set, 0, strlen($set) - 1);


    $wherefields = array_keys($where);
    $whereCondition="";
    foreach ($wherefields as $field) {
        $whereCondition .= " $field = :$field AND";
    }
    //strip last AND
    $whereCondition = substr($whereCondition, 0, strlen($whereCondition) - 3);

    $sql = "UPDATE $table SET $set WHERE $whereCondition";
    var_dump($sql);

    $stmt = $this->db->prepare($sql);

    foreach ($values as $field => $value) {

        $stmt->bindParam(':$field', $value);
    }
    foreach ($where as $field => $value) {
        $stmt->bindParam(':$field', $value);
    }
    return $stmt->execute();
}

The problem is all the fields in the record are being updated by the id of the record which is contained in the $where variable.

$values contains an array of (fieldname=>value).

I think the problem lies around the bindparam and trying to make the fieldnames/placeholders dynamic

I thought I needed to use bindparam as best practice - is this correct or can I just go to execute()?

ANy help appreciated

Can you write what the resulting SQL looks like? – Jose Armesto Mar 27 '13 at 09:01 — Jose Armesto, Mar 27 '13 at 09:01

score 2 · Accepted Answer · edited May 23 '17 at 11:49

You are lifting this log from the wrong end.
Your approach is potentially insecure yet inflexible at the same time. What if you need a JOIN based update? What if you need OR in the WHERE (or IN)?

What you really need is a conventional query where only SET statement values have to be generated. So, you need a helper function to produce such a statement out of data array, returning both correctly formatted SET statement and array with variables to be bound:

$fields = array("name","email");
$sql = "UPDATE users SET ".pdoSet($fields,$values,$data)." WHERE id = :id"
// now we have $values array to be passed into query
$stmt = $dbh->prepare();
$values["id"] = $_POST['id'];
$stmt->execute($values);

With this code you'll be able to make updates for the arbitrary query. And make it safe.

As a further step you will need to start using type-hinted placeholders, to make whole code like this:

$db->query("UPDATE ?n SET ?u WHERE id IN(?a)",$table,$data,$ids);

Getting back to your problem, ONe is right - you need to use bindValue instead of bindParam (as it mentioned in the tag wiki)

I updated to bindvalue to solve the current problem. I must admit to being naive about creating helper functions so one for my learning list — Ray, Apr 08 '13 at 09:43

score 1 · Answer 2 · edited May 23 '17 at 12:21

I believe the problem is that you are using a foreach to bind the params to the query. Why is this a problem? Because when you bind a variable, you bind a reference to that variable, so if that variable changes, the value in the query will change too. Since you are using a foreach loop, the value for all the parameters will be the latest value that the variable $value referenced to.

You can read more about this foreach behavior here and here. So basically, you have 2 options:

Use a reference to the actual value, instead of using a reference to $value (which can change its value in the next iteration)
Use an auxiliar variable that references another memory position that won't change during the loop

score 0 · Answer 3 · answered Jul 09 '14 at 20:50

I came here because I was having the same problems, and YCS's solution was what I needed. For anyone else in this situation, here's the helper function I ended up using:

function commit_table($record_id, $changed_values) { $db = open_database();

$query = 'UPDATE table SET ';
$query_arguments = array();

$is_first = TRUE;

foreach(array_keys($changed_values) as $key)
{
    if($is_first)
    {
        $is_first = FALSE;
    }
    else
    {
        $query .= ', ';
    }
    $value_var = ':' . $key;
    $query .= $key;
    $query .= ' = ';
    $query .= $value_var;
    $query_arguments[$value_var] = $changed_values[$key];
}

$query .= ' WHERE record_id = :record_id';
$query_arguments[':record_id'] = $record_id;

$stmt = $db->prepare($query);
$stmt->execute($query_arguments);

close_database($db);

}

creating a flexible update query with Php and pdo - problems with bindparam

3 Answers3