CODEWITHSUNDEEP
phpmysqlooppdo
bsteo
bsteo

Reputation: 1779

PHP PDO MySQL count() prepared statement

I'm on a Web development course. Working with PHP PDO MySQL they teach us on a workshop to do this:

function countUsers($search) {
    $and = '';
    if ($search != '') {
        $and = " AND user_name LIKE '%".$search."%'";    
    }
    $total = $this->db->query("SELECT COUNT(id) as rows FROM users WHERE valid = 1" . $and)->fetch(PDO::FETCH_OBJ);
    return $total->rows;
}

From my point of view this is totally wrong, the statement is not prepared and is passed directly from user input without any validation that can lead to SQL Injection, so I proposed this to the trainer (I know fetchColumn() would be more appropriate here but let's stick with this for the sake of the example):

function countUsers($search) {
    $and = '';
    $sqlSearch = "%$search%";

    if ($search != '') {
        $and = " AND user_name LIKE :username";  
    }

    $sql = "SELECT COUNT(id) as rows FROM users WHERE valid = 1" . $and;
    $sth = $this->db->prepare($sql);
    if ($search != '') {
        $sth->bindParam(':username', $sqlSearch, PDO::PARAM_STR);
    }
    $sth->execute();
    $total = $sth->fetch(PDO::FETCH_OBJ);
    return $total->rows;
}

Am I wrong? Are they wrong or we both wrong/right?

Upvotes: 0

Views: 973

Answers (1)

Your Common Sense
Your Common Sense

Reputation: 157895

Yes you are right.

However, your code is not optimal. In fact, prepared statements are intended to make your code cleaner, not more bloated.

function countUsers($search) {
    $sql = "SELECT COUNT(id) FROM users WHERE valid = 1 AND user_name LIKE ?";
    $sth = $this->db->prepare($sql);
    $sth->execute(["%$search%"]);
    return $sth->fetchColumn();
}

Part of the cleanup I did is a mere trick - as you can always search for LIKE '%%' and match all rows (excluding ones where user_name is null though).

But the rest is just a proper use of PDO features:

  • you can always use positional placeholders
  • you can always avoid bindParam() call
  • you should use appropriate fetch mode

Upvotes: 4

Related Questions