Question

I've written the following function to construct and execute an SQL statement with key-value bindings. I'm using bindValue() to bind an array of key-value pairs to their corresponding identifiers in the SQL string. (The echo statements are for debugging).

public function executeSelect($sql, $bindings = FALSE)
{
    $stmt = $this->dbPDO->prepare($sql);

    if ($bindings)
    {
        foreach($bindings as $key => $value)
        {
            $success = $stmt->bindValue($key, $value);
            echo "success = $success, key = $key, value = $value<br />";
            if (!$success)
            {
                throw new Exception("Binding failed for (key = $key) & (value = $value)");
            }
        }
    }

    echo "Beginning execution<br />";
    if ($stmt->execute())
    {
        return $stmt->fetchAll(PDO::FETCH_ASSOC);
    }
    else
    {
        return FALSE;   
    }
}

The input to this function is as follows:

$stmt = "SELECT * FROM test WHERE id = :id";
$bindings = array(":id" => "3", ":Foo" => "Bar");

On the second loop through the $bindings array, I'd expect $success to evaluate to false, thus throwing the custom Exception since "Bar" cannot be bound to ":Foo", since ":Foo" doesn't exist in the input SQL.

Instead, $success evaluates to true (1) for both key-value pairs in the $bindings array, and a PDOException is thrown by ->execute() "(HY000)SQLSTATE[HY000]: General error: 25 bind or column index out of range"

Why isn't bindValue returning false?

Was it helpful?

Solution

Because it works this way.
it throws an error not at bind but at execute. That's all.

So, there is no need in loop and you can make your method way shorter.

public function executeSelect($sql, $bindings = FALSE)
{
    $stmt = $this->dbPDO->prepare($sql);
    $stmt->execute($bindings);
    return $stmt->fetchAll(PDO::FETCH_ASSOC);
}

There is no need in checking execute result either, I believe.
In case of error it will raise an exception already.

By the way, I'd make several helper functions based on this one, returning scalar value and single row. They are mighty helpful. Though I find named placeholders a bit dull. Compare this code:

$name = $db->getOne("SELECT name FROM users WHERE group=?i AND id=?i",$group,$id);
vs.
$sql = "SELECT name FROM users WHERE group=:group AND id=:id";
$name = $db->getOne($sql,array('group' => $group, 'id' => $id));

named require 2 times more code than anonymous.
A perfect example of WET acronym - "Write Everything Twice"

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top