Is it bad practice to use variable inside pdo query?

Question

I have a PHP PDO query like this:

$update = $pdo->query("UPDATE login_users SET lastactivity = '$lastactivity' WHERE user_id = 1");

I like to do this way because I think my code looks better.

Is it ok to do this way? Or should I do like this:

$params = array(
                     ':id'       => $_SESSION['jigowatt']['user_id'],
                     ':lastactivity'       => $lastactivity,
                 );

         $update = $pdo->query('UPDATE login_users SET lastactivity = :lastactivity WHERE user_id = :id', $params);

See [how-can-i-prevent-sql-injection-in-php](https://stackoverflow.com/questions/60174/) — juergen d, Oct 27 '16 at 22:20
*"Is it bad practice to use variable inside pdo query?"* - no. *"Is it bad practice to use **an un-sanitized** variable inside pdo query?"* very yes — Memor-X, Oct 27 '16 at 22:24
Is it safe this way? `$sql = "UPDATE login_users SET lastactivity = ? WHERE user_id = ?"; $pdo->prepare($sql)->execute([$lastactivity, 1]);` — Vinny, Oct 27 '16 at 22:33
Because I found an example on Google, and wrote my script like this, so I wanted to be sure I did right.. https://phpdelusions.net/pdo#dml good to know it's safe.. — Vinny, Oct 27 '16 at 22:36
I see. For some reason the example in your question is different, hence the confusion — Your Common Sense, Oct 27 '16 at 22:45

Your Common Sense · Accepted Answer · 2016-10-27T22:45:56.177

I have a PHP PDO query like this:

By no means you should have code like this.

Or should I do like this

Neither you are bound to write a prepared statements like that. There are other ways. For example, nobody's forcing you to use named placeholders. You can use positional, they are much more concise:

$sql = "UPDATE login_users SET lastactivity = ? WHERE user_id = ?";
$pdo->prepare($sql)->execute([$lastactivity],$_SESSION['jigowatt']['user_id']);

Deep · Answer 2 · 2016-10-27T22:39:16.870

0

This is example of PDO "query wrapper":

public function sendQuery($queryString, array $queryParams = array())
{
    try {
        $stmt = $this->_PDO->prepare($queryString);
        $stmt->execute($queryParams);
    } catch (\PDOException $e) {
        if ($this->_PDO->inTransaction()) {
            $this->_PDO->rollBack();
        }
        throw $e; // this line for example only (see comments of this answer)
    }

    return $stmt;
}

Can be use like this:

$conn = \DBI::getConnection(); // DBI return DBC object
$x = $conn->sendQuery(
    'SELECT x FROM tbl WHERE y = :y',
    [':y' => $y]
)->fetch(\PDO::FETCH_COLUMN);

edited Oct 27 '16 at 22:39

answered Oct 27 '16 at 22:29

Deep

2,472
2
15
25

1

as long as you don't have any custom exception, just throw the one you have – Your Common Sense Oct 27 '16 at 22:31
@YourCommonSense author can use custom exception and/or something else (rethrow). And I comment this line. – Deep Oct 27 '16 at 22:32
1

Yes they can. I am not arguing that. But if they don't, there will be no exception thrown at all. So you have to keep it throwing existing exception by default – Your Common Sense Oct 27 '16 at 22:33
@YourCommonSense by default of what? PSR? – Deep Oct 27 '16 at 22:35
1

of common sense – Your Common Sense Oct 27 '16 at 22:39
2

In effect, the code you wrote at first **didn't report any errors at all**. Which makes it HELL when it comes to debugging. By no means it should be any default behavior. – Your Common Sense Oct 27 '16 at 22:42
@YourCommonSense I disagree. Each person have personal right common sense. Some of them similar. But other no. Otherwise, there would be so many different frameworks and CMS. – Deep Oct 27 '16 at 22:42
2

There are basic rules, common for everyone. Error reporting is one of them. – Your Common Sense Oct 27 '16 at 22:43
@YourCommonSense error reporting this is not exceptions. Exceptions can be as reports. But reports can't be as exceptions. – Deep Oct 27 '16 at 22:46
So what? Does it excuse writing a code that disables error reporting at all? – Your Common Sense Oct 27 '16 at 22:47
I write "throw new YourCustomException" your fix it to "throw $e" What different? Not example something of "throw new YourCustomException(['title' => 'Database error', 'description' => $e->getMessage()]);" ? I don't known about application architecture of author. PDO exceptions can be different with internal exceptions (instanceof, constructor args) – Deep Oct 27 '16 at 22:51
"throw new YourCustomException" sounds cryptic. Not everyone is using custom exceptions. Yet people tend to copy and paste the code from answers. So it's better to write a code that's ready to use. You see, it's better to make it work properly by default. It's all about defaults actually :) – Your Common Sense Oct 27 '16 at 22:53
@YourCommonSense and all of this we explain in this comments )) – Deep Oct 27 '16 at 22:55
@YourCommonSense this is about us https://www.youtube.com/watch?v=_MNNl-oOI7I – Deep Oct 27 '16 at 23:00

Is it bad practice to use variable inside pdo query?

2 Answers2