PHP + MySQL Issue - where did I go so wrong?

Question

Here's the dig. I've looked over and over my code until my mind was numb and still can't figure out where the issue is.

I'm working with a delete feature that works on the backend of a CRM-like application. There is a user list with checkboxes by each registered user inside of a form, and a submit button to run the code to delete the selected users from the database by their ID.

The checkboxes are generated in a foreach loop that populates the table using an array returned from a select query. Each checkbox line for each user is as such:

<input type="checkbox" name="checked[]" value="<?php echo ($userfromforeach['id']); ?>">

The submit button for the form has (name="deleteusers") included.

And the actual delete code for the form submission is as follows:

// Delete users by selected as posted by form
if(isset($_POST['deleteusers']))
{
    foreach($_POST['checked'] as $user)
    {
        $query = "
            DELETE
            FROM $usertable
            WHERE id = $user
        ";

        try
        {
            // These two statements run the query against your database table.
            $stmt = $db->prepare($query);
            $stmt->execute();
        }
        catch(PDOException $ex)
        {
            die("MySQL execution error, please contact technical support.");
        }

    }
    // $_POST['checked'] = NULL;
    header("Refresh:0");
}

For some reason unknown to me, everything seems to be firing correctly however no records are being deleted from the database. No MySQL error is returned, and when I print_r out the $_POST['checked'] variable it shows as would be expected:

Array ( [0] => 122 [1] => 115 )

^ The values to each key being the user's ID.

Any thoughts? Have I just knuckle-headed up a problem in my own head by missing something entirely elementary? Thanks very much for your help.

Echo out the SQL you're generating - does it look OK. Also - you are not using prepared statements properly. The idea is that you prepare the statement once with placeholders, and then execute it multiple times, passing in different parameters each time. — andrewsi
– andrewsi, Commented Jun 16, 2016 at 13:24
Echoing out the SQL does look as it should also... I know, that I'm not using prepared statements properly - I'm rather new to PHP and this is a demo which I have a time limit on, the prepared statements confused me so for a non-production demo I decided it would be fine to run the queries unprepared. - Thanks for the heads up though. Sorry about that, typo - edited in above in the question. — John Doe
– John Doe, Commented Jun 16, 2016 at 13:29
echo $query = "DELETE FROM $usertable WHERE id = $user"; and paste what it return — Saty
– Saty, Commented Jun 16, 2016 at 13:34

Rahul M · Accepted Answer · 2016-06-16 13:38:34Z

3

When you loop over something with for/foreach functions. You've to wrap the whole repeating logic with-in curly braces. Otherwise ONLY ONE SINGLE STATEMENT WILL BE EXECUTED which in your case is assigning a string to the $query variable. And that's it. Though there are still a number of improvements to be made. But to solve this particular issue, your foreach should look like following:

foreach($_POST['checked'] as $user)
{
    $query = "DELETE FROM $usertable WHERE id = $user";
    try
    {
        $stmt = $db->prepare($query);
        $stmt->execute();
    }
    catch(PDOException $ex)
    {
        die("MySQL error, blah blah blah...");
    }
}

answered Jun 16, 2016 at 13:38

Rahul M

1,50912 silver badges19 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Jwags Over a year ago

I missed that too. Good catch!

John Doe Over a year ago

I'm sorry my friend - I've been writing this in a VM running debian and for the moment the Guest-Host clipboard isn't working, so I was unable to copy-paste my code over. The curly braces are indeed covering the entire loop statement - I've edited the question appropriately.

Ashish Nayyar · Accepted Answer · 2016-06-16 13:54:22Z

2

Why running delete query in a loop, what if user selects 200 records? You will fire query 200 times?

I recommend you write your PHP login in such a way that it satisfying the following queries

DELETE from tablename WHERE id IN (1,2,3,...,200);

Use BETWEEN if you have consecutive IDs :

DELETE from tablename WHERE id BETWEEN 1 AND 200;

Limiting only for some IDs :

DELETE from tablename WHERE id BETWEEN 1 AND 200 AND id<>47;

This way the query will be fired only once, provided the input for the MySQL query (IDs) are pre-generated via PHP loop.

answered Jun 16, 2016 at 13:54

Ashish Nayyar

5603 silver badges10 bronze badges

1 Comment

John Doe Over a year ago

You're right, my code is inefficient. Not terribly so because the maximum number of queries to be fired at a single time would only be 25 but still something that should be better streamlined.

Naisa purushotham · Accepted Answer · 2016-06-16 13:49:35Z

1

if(isset($_POST['deleteusers'])) <-- submit button with the name "deleteusers"
    {   

        foreach($_POST['checked'] as $user)
        {
            $query = "DELETE FROM $usertable WHERE id = :user";     
            try
            {
                $stmt = $db->prepare($query);
                $stmt->bindParam(':user', $user);
                $stmt->execute();
            }
            catch(PDOException $ex)
            {
                die("MySQL error, blah blah blah...");
            }
        }
    }

and you can run this one in one statement using this query

$query = "DELETE FROM $usertable WHERE id IN (:user)";

$stmt->bindParam(':user',implode(",",$_POST['deleteusers']));

edited Jun 16, 2016 at 13:49

answered Jun 16, 2016 at 13:43

Naisa purushotham

91310 silver badges19 bronze badges

2 Comments

John Doe Over a year ago

Wouldn't that actually be $query = "DELETE FROM $usertable WHERE id IN (".implode(",",$_POST['checked']).")"; ? The deleteusers variable is set by clicking the submit button, whereas the checked variable contains the array of user ID's to be deleted...

Naisa purushotham Over a year ago

after implode result query was $query = "DELETE FROM $usertable WHERE id IN (122,115)"; this is normal delete query

John Doe · Accepted Answer · 2016-06-16 14:56:05Z

Well I'm an idiot.

After much digging it turns out that this issue is that my submit button in bootstrap's modal popup box, and apparently even though by my understanding, modal only essentially hides and shows a div in the page, any post data from the modal div won't get passed back to the main page. >:[ As such, the only solution I can think of is to set the $_POST['deleteusers'] variable from javascript on click of the delete button... What a pain in the sitter...

In any case, thank you all for your help, I will mark The Bigbyte Number's answer because he covered two other very important considerations - using prepared statements correctly and how to streamline the process into one single query, saving resources and theoretically CPU time as well.

Collectives™ on Stack Overflow

PHP + MySQL Issue - where did I go so wrong?

4 Answers 4

2 Comments

1 Comment

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

1 Comment

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related