7

I know this has been asked 1000 times, but for some reason I continue to bang my head agains the wall..

This works:

$sql = 'SELECT a.eventCode, a.eventTime, a.teamCode, a.playerCode, b.lastName, b.firstName, b.number, a.xCoord, a.yCoord, a.id ';
$sql = $sql . 'FROM events a, players b ';
$sql = $sql . 'WHERE a.regGUID in ( ' . $regGUID . ' ) and ';
$sql = $sql . 'a.playerCode=b.playerCode and a.gameCode = "' . $game . '" order by a.eventTime desc, a.actionCode asc'; 
$stmt = $db->prepare($sql);
$results = $stmt->execute();

This Doesn't:

$sql = 'SELECT a.eventCode, a.eventTime, a.teamCode, a.playerCode, b.lastName, b.firstName, b.number, a.xCoord, a.yCoord, a.id ';
$sql = $sql . 'FROM events a, players b ';
$sql = $sql . 'WHERE a.regGUID in ( :regGUID ) and ';
$sql = $sql . 'a.playerCode=b.playerCode and a.gameCode = :game order by a.eventTime desc, a.actionCode asc'; 
$stmt = $db->prepare($sql);
$stmt->bindValue(':regGUID', $regGUID, PDO::PARAM_STR);
$stmt->bindValue(':game', $game, PDO::PARAM_STR);
$results = $stmt->execute();

What am I missing? Thanks

4
  • is regGUID really a string?
    – hek2mgl
    Commented Jan 19, 2013 at 17:01
  • 1
    Make sure that you can see PDO errors by changing from its default silent error mode: $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING)
    – eggyal
    Commented Jan 19, 2013 at 17:01
  • 4
    Set PDO to throw exceptions on errors, see if it throws anything. Is $regGUID a single GUID, or a comma-separated list of GUIDs? If the latter, each GUID has to be bound as a separate variable.
    – user1233508
    Commented Jan 19, 2013 at 17:02
  • 1
    Advice to check for query errors is good, but in this case this query won't throw an error -- it will just fail to match any rows, because no individual GUID string in the database is equal to a string which is a comma-separated list of GUIDs. Commented Jan 19, 2013 at 17:21

4 Answers 4

4

The problem is here:

$sql = $sql . 'WHERE a.regGUID in ( :regGUID ) and ';
$stmt->bindValue(':regGUID', $regGUID, PDO::PARAM_STR);

I assume $regGUID is a comma-separated list of quoted strings.

Each query parameter accepts only a single scalar value. Not lists of values.

So you have two choices:

  1. Continue to interpolate the $regGUID string, even if you use parameters for other scalar values. But you still want to be careful to avoid SQL injection, so you must form the $regGUID string correctly. You can't just call PDO::quote() on the whole string, that would make it a single quoted string containing UUIDs and commas. You have to make sure each UUID string is escaped and quoted individually, then implode the list together and interpolate it into the IN clause.

    $regGUIDs = explode(',', $regGUID);
    $regGUIDs = array_map(function ($g) { return $db->quote($g); }, $regGUIDs);
    $regGUID = implode(',', $regGUIDs);
    $sql = $sql . 'WHERE a.regGUID in (' . $regGUID . ') and ';
    
  2. explode() the $regGUID into an array, and add one query parameter for each element in the array. Interpolate the dynamic list of query parameter placeholders.

    $regGUIDs = explode(',', $regGUID);
    $params = array_fill(1, count($regGUIDs), '?');
    $sql = $sql . ' WHERE a.regGUID in ( ' . implode(',', $params) . ' ) and ';
    

You could bindValue() in a loop for the array, but keep in mind that other parameters should also be bound by position, not by name. PDO has bugs that make it not happy when you try to mix the two different styles of parameters in the same query.

Instead of using bindValue() I just pass an array of parameter values to PDOStatement::execute(), which is much easier.

$paramValues = $regGUIDs;
$paramValues[] = $game;
$results = $stmt->execute($paramValues);
2
  • Thank you.. Worked like charm! Small typo correction in the implode function, took a couple of minutes to realize there should have been a , instead of a . My head hurts less from banging against the wall! Commented Jan 19, 2013 at 17:50
  • Thanks, I have corrected the , in my answer above. Glad to help! Commented Jan 19, 2013 at 19:43
2

This indeed has been asked 1000 times.

Prepared statements can only accept scalar values, not arbitrary parts of the SQL query.

You have to form IN() statement using as many placeholders, as many items you have to put in and then bind them one by one.

To ease this task one can use some helper function.

Say, using SafeMysql library this code could be written as

$sql  = 'SELECT * FROM events a, players b WHERE regGUID in (?a) and';
$sql .= ' a.playerCode=b.playerCode and a.gameCode = ?s';
$sql .= ' order by a.eventTime desc, a.actionCode asc'; 
$results = $db->getAll($sql,$regGUID,$game);

Note that $regGUID should be an array, not string and $results already contain all the requested data, without any further processing.

1

What is the content of $regGUID? Since you're using an in clause, I suspect a comma separated list.

Binding a variable to a parameter is not like substituting that string into the query; it is like telling MySQL how to use your actual PHP variables. So if you bind a string like '1,2,3' to the query parameter, it stays as one string, and is not reinterpreted as a list of numbers.

Consequently, if $regGUID is something like "'AAA1', 'BBB2'", your first query becomes

... WHERE a.regGUID in ( 'AAA1', 'BBB2' ) ...

but your second query is more like

... WHERE a.regGUID in ( '\'AAA1\', \'BBB2\'' ) ...

which is the same as saying

... WHERE a.regGUID = '\'AAA1\', \'BBB2\'' ...
1

As others have state you can only bind a single scalar value to a placeholder. So this means you actually need a placeholder for each value in your IN statement. I normally do something like the following. It should be noted though that i never use bindValue so if it has rules about things having to be references like Mysqli then the below may need to be modified:

$regGUIDPlaceholders = array();

// prepare the placeholders
// assume regGUID is an array - if its a string then explode on whatever to make it an array
foreach($regGUID as $k => $v) {
   $placeholder = ':regGUID' . $k;
   $regGUIDPlaceholders[$key] = $value;
}

// prepare the IN statememnt
$in = sprintf('IN (%s)', implode(',', array_keys($regGUIDPlaceholders)));

$sql = 'SELECT a.eventCode, a.eventTime, a.teamCode, a.playerCode, b.lastName, b.firstName, b.number, a.xCoord, a.yCoord, a.id ';
$sql = $sql . 'FROM events a, players b ';

// USE the IN statement dynamically prepared above
$sql = $sql . 'WHERE a.regGUID '. $in . ' and ';

$sql = $sql . 'a.playerCode=b.playerCode and a.gameCode = :game order by a.eventTime desc, a.actionCode asc'; 

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

// bind each GUID to its placeholder
foreach($regGUIDPlaceholders as $placeholder => $value) {
   $stmt->bindValue($placeholder, $value, PDO::PARAM_STR);
}

$stmt->bindValue(':game', $game, PDO::PARAM_STR);
$results = $stmt->execute();

Not the answer you're looking for? Browse other questions tagged or ask your own question.