PHP implode array to generate mysql IN criteria

Question

I have a function like the following:

public function foo ($cities = array('anaheim', 'baker', 'colfax') )
{
    $db = global instance of Zend_Db_Adapter_Pdo_Mysql...

    $query = 'SELECT name FROM user WHERE city IN ('.implode(',',$cities).')';
    $result = $db->fetchAll( $query );
}

This works out fine until someone passes $cities as an empty array.

To prevent this error I have been logic-breaking the query like so:

$query = 'SELECT name FROM user';
if (!empty($cities))
{
    $query .= ' WHERE city IN ('.implode(',',$cities).')';
}

but this isn't very elegant. I feel like there should be a better way to filter by a list, but I am not sure how. Any advice?

This will also break if a user submits a city that contains `'` (SQL injection) or a comma... You need to wrap them in `'` s — Pekka, Oct 05 '10 at 21:24
It is perfectly fine to do it this way. If there are no cities, you don't need a `WHERE` clause... — Felix Kling, Oct 05 '10 at 21:25
Looks fine to me, but you could use one of the many classes out there for making nice prepared queries. At the very least, escape your strings!! http://php.net/manual/en/function.mysql-real-escape-string.php — Brad, Oct 05 '10 at 21:26
Apart from the concerns Pekka mentions, I see no reason why tagging on a `WHERE` clause depending on input would be inelegant. IMHO it's nicer then getting a meaningless `WHERE` clause that just means 'all'. However, even for variable-count IN's I still prefer prepared statements (usually using a `implode(',',array_fill(0,count($args),'?')`. — Wrikken, Oct 05 '10 at 21:30
No, don't use mysql_real_escape_string. If $db is an instance of Zend_DB use *its* methods to either encode the query properly or to build a prepared, parametrized query. And concerning the where clause: Do you really _want_ your method foo() to do both, select some citites and select all cities? Personally I don't like that, those are two separate concerns -> two methods. — VolkerK, Oct 05 '10 at 21:30

score 7 · Answer 1 · answered Oct 06 '10 at 19:15

If you do end up using the select object the ->where() method will actually handle arrays for you. Still need to check to see if there are items in the array, but this makes it a cleaner approach...

$select = $db->select()->from('user', 'name');

if ($cities) {
    $select->where('city IN (?)', $cities);
}

score 3 · Accepted Answer · answered Oct 06 '10 at 03:26

At least use the quote method...

if ($cities) {
    $query .= sprintf('WHERE city IN (%s)', implode(',', array_map(array($db, 'quote'), $cities)));
}

or, ideally, construct the query with Zend_Db_Select...

$select = $db->select()->from('user', 'name');

if ($cities) {
  foreach ($cities as $city) {
        $select->orWhere('city = ?', $city);
    }
}

score 1 · Answer 3 · answered Mar 26 '12 at 22:45

So you know, from Zend Docs of Zend_Db_Adapter::quote "If an array is passed as the value, the array values are quoted * and then returned as a comma-separated string."

So you could do this which is also quoted properly:

if ($cities) $query .= 'WHERE city IN ({$db->quote($cities}) ';

I love 1-liners :)

PHP implode array to generate mysql IN criteria

3 Answers3

Linked