Deletion of multiple fields fail

Question

I want to be able to delete multiple users from the database, but the code below fails in some point. What happens is that only the last clicked user (i.e. the last element in the array $userIds) gets deleted.

What am I doing wrong?

from UserModel.php:

public function RemoveUser(Array $userIds) {

    $query = 'DELETE FROM Users WHERE id IN (?)';

    $stmt = $this->m_db->Prepare($query);

    foreach ($userIds as $value) {
        $stmt->bind_param('s', $value);
    }

    $ret = $this->m_db->DeleteUsers($stmt);

    $stmt->Close();

    return $ret;

}

from Database.php:

public function DeleteUsers(\mysqli_stmt $stmt) {

    if ($stmt === FALSE) {
        throw new \Exception($this->mysqli->error);
    }

    if ($stmt->execute() == FALSE) {
        throw new \Exception($this->mysqli->error);
    }

    if ($stmt->fetch()) {
        return true;
    } else {
        return false;
    }

}

it looks like you're binding n params to a single slot in the query `?`, is that normal? — Sebas, Oct 07 '12 at 14:20
You need to construct the SQL string dynamically, and add a `?` for each array element. I'm looking for a question that addresses this with a good answer... — Michael Berkowski, Oct 07 '12 at 14:21
@MichaelBerkowski: Ah, I see the problem now. But how could the SQL statement be created dynamically? — holyredbeard, Oct 07 '12 at 14:24
@holyredbeard See http://stackoverflow.com/questions/3703180/a-prepared-statement-where-in-query-and-sorting-with-mysql. I'll post a better example if I can find one - I know it's out there. — Michael Berkowski, Oct 07 '12 at 14:29
What you are passing is not a parameter but a part of the query syntax. you can do it in two ways, the ugly and not recommended one is to use the string joins and construct the sql query. Another is, prepare the query dynamically. I am looking for a good answer so I can give you reference to. — Murtuza Kabul, Oct 07 '12 at 14:29

Dan Simon · Answer 1 · 2012-10-07T18:25:19.807

3

As some of the comments suggest, you need to have a ? for each user id. Currently, you're attempting to bind each user id to the same parameter, so only the last one will actually apply.

$c = Array();
foreach ($userIds AS $u) {
  $c[] = "?";
}
$inPart = "(" . implode(",", $c) . ")";
$query = "DELETE FROM Users WHERE id IN $inPart";

Since bind_param expects each variable as a separate argument, you'll have to do a little php magic to pass the whole array at once. You'll have to change your binding loop to:

call_user_func_array(array($stmt, 'bind_param'), array_unshift($userIds, 's'));

This basically calls $stmt->bind_param('s', $userIds[0], $userIds[1]....)

edited Oct 07 '12 at 18:25

answered Oct 07 '12 at 14:32

Dan Simon

12,891
3
49
55

I don't see $u is being used in the foreach loop? – holyredbeard Oct 07 '12 at 14:39
It doesn't need to be, I'm just looping over the array to create enough parameters. – Dan Simon Oct 07 '12 at 15:41
Ok, I see that now. But what about my code where i'm looping through the ids; foreach ($userIds as $value) { $stmt->bind_param('s', $value); }...? When implementing your code that piece of code generates error (Warning: mysqli_stmt::bind_param() [mysqli-stmt.bind-param]: Number of variables doesn't match number of parameters in prepared statement in /Applications/XAMPP/xamppfiles/htdocs/php-labbar/hp-php/lab3/login/Model/UserHandler.php on line 45) – holyredbeard Oct 07 '12 at 15:44
Oh, I see you're using mysqli and not PDO, I've updated my answer. – Dan Simon Oct 07 '12 at 18:25
Tried it but gets the following error: "Warning: call_user_func_array() expects parameter 2 to be array, integer given in /Applications/XAMPP/xamppfiles/htdocs/php-labbar/hp-php/lab3/login/Model/UserHandler.php on line 44" – holyredbeard Oct 07 '12 at 20:53

score 2 · Answer 2 · edited May 23 '17 at 11:58

I modified Dan Simons answer and managed to get this working.

One of the problems with this solution is that the 2nd parameter in call_user_func_array needs to be referenced, with this question is about. The problem is however solved by using the function makeValuesReferenced.

The code:

public function RemoveUser(Array $userIds) {

    $c = Array();
    $s = '';

    foreach ($userIds AS $u) {
        $c[] = "?";
        $s.= 's'; 
    }

    $inPart = "(" . implode(",", $c) . ")";
    $query = "DELETE FROM Users WHERE id IN $inPart"; 

    $stmt = $this->m_db->Prepare($query);

    array_unshift($userIds, $s);

    call_user_func_array(array($stmt, 'bind_param'), $this->makeValuesReferenced($userIds));

    $ret = $this->m_db->DeleteUsers($stmt);    // Execution and fetching

    $stmt->Close();

    return $ret;

}

public function makeValuesReferenced(Array $arr) {
        $refs = array();

        foreach($arr as $key => $value)
            $refs[$key] = &$arr[$key];
        return $refs;

}

Deletion of multiple fields fail

2 Answers2