0
\$\begingroup\$

I have four mysql tables setup like the following:

users (userid, firstname, lastname)  *one entry per user
schedules (userid, scheduleid, schedulename) *each user could have multiple schedules
classes (classid, scheduleid, classname) *each schedule is made up of multiple classes
schools (scheduleid, schoolid, schoolname) *one entry per user

I want the user to be able to delete their account, including ANY info from other tables tied to their account. So if they had 3 schedules with a bunch of classes in each and a school tied to each schedule, I'd want ALL of that data (including their user info) to be deleted in one fell swoop.

userid is all I need to delete from the users and schedules table easily. But the classes and schools tables DON'T have a userid field, so I have to do some other way.

I wrote and tested the following code and it works fine...

//first get all schedules associated with this user account and put scheduleids into array
$sql = "SELECT scheduleid FROM schedules 
        WHERE userid = '$userid'"; 
        $selectschedules = mysqli_query($connection, $sql);
        if (!$selectschedules) {
        die("Database query failed: " . mysqli_error());
        } else {
            while($data=mysqli_fetch_assoc($selectschedules)){
                 $schedules[] = $data;
            }
        }
//now loop through each schedule to delete all classes and any school associated with that particular schedule  
foreach ($schedules as $item) {
    $sql = "DELETE FROM classes 
            WHERE scheduleid = '".$item['scheduleid']."'"; 
            $deleteclasses = mysqli_query($connection, $sql);
            if (!$deleteclasses) {
            die("Database query failed: " . mysqli_error());
            } else {
            }
    $sql = "DELETE FROM schools
            WHERE scheduleid = '".$item['scheduleid']."'";
            $deleteschool = mysqli_query($connection, $sql);
            if (!$deleteschool) {
            die("Database query failed: " . mysqli_error());
            } else {    
            }
}
//next delete all schedules associated with user account
$sql = "DELETE FROM schedules 
        WHERE userid = '$userid'"; 
        $deleteschedules = mysqli_query($connection, $sql);
        if (!$deleteschedules) {
        die("Database query failed: " . mysqli_error());
        } else {
        }
//finally, delete the user's account
$sql = "DELETE FROM users 
        WHERE userid = '$userid'
        LIMIT 1"; //should only be one, just being safe
        $deleteuser = mysqli_query($connection, $sql);
        if (!$deleteuser) {
        die("Database query failed: " . mysqli_error());
        } else {    
        }

However, I believe the above code is inefficient (particularly looping through each schedule and doing delete queries on every loop). So I'm curious if there is a shorter/more efficient way to accomplish this same thing. I am not good with JOINS and could be wrong, but would using JOINS potentially be the key to shortening this and NOT having to loop at all?

SIDE NOTE: Earlier I mentioned that classes and schools tables DON'T have a userid field. If they did, it would be very easy to delete all of this stuff in one query. So it begs the question: Should I add the userid field to the other tables? I don't want to add it just to make deleting easier, but if you think it SHOULD have it regardless, then I would consider.

Any info/suggestions would be greatly appreciated.

\$\endgroup\$
3
\$\begingroup\$

Basically there are three vectors of improvement:

  • of course you can make your queries optimal, avoiding a loop
  • also, I would suggest to optimize the the process of executing the query, as it is evident that each query takes too much code to write
  • you should never put a single variable directly in the query, but always use a prepared statement instead.

To solve the first problem, you can always use a JOIN. Yes, DELETE queries support JOINs as well as SELECT ones. So it goes:

DELETE c.* FROM classes c, schedules s WHERE c.scheduleid=s.id AND s.userid = ?

To solve the second one would propose to use PDO (which can solve the third one as well) as it lets you to run a query in one line:

$pdo->prepare("DELETE FROM users WHERE userid = ?")->execute([$userid]);

So the overall code would be

$sql = "DELETE c.* FROM classes c, schedules s WHERE c.scheduleid=s.scheduleid AND s.userid = ?";
$pdo->prepare($sql)->execute([$userid]);

$sql = "DELETE c.* FROM schools sc, schedules s WHERE sc.scheduleid=s.scheduleid AND s.userid = ?";
$pdo->prepare($sql)->execute([$userid]);

$sql = "DELETE FROM schedules WHERE s.userid = ?";
$pdo->prepare($sql)->execute([$userid]);

$sql = "DELETE FROM users WHERE s.userid = ?";
$pdo->prepare($sql)->execute([$userid]);

As long as you are connecting to PDO properly, this code will report errors automatically, without a single extra line of code!

Should I add the userid field to the other tables?

Quite contrary, you should NOT add such a field, keeping your database structure as normalized as possible

\$\endgroup\$
  • \$\begingroup\$ This looks like great info thank you. Ill definitely look into PDO. I have a ton of existing queries, sounds like you're saying it would be worth the time to go back and change all of them to use PDO? Finally, your way is much less code, but I'm just curious, there is still no way to delete everything from all four tables in literally ONE query using JOINs? I'm guessing you gave it to me as succint as it can be, just curious. Thx! \$\endgroup\$ – user3304303 May 19 '17 at 14:48
  • 1
    \$\begingroup\$ I think yes, you can delete with a single query, but I never tried it myself. But it is supported - you just add the table alias after delete, like DELETE c, sc, u FROM .... You can experiment yourself and then post an answer! \$\endgroup\$ – Your Common Sense May 19 '17 at 14:58
  • 1
    \$\begingroup\$ I should have mentioned a cascade delete also. Actually you can set foreign keys in mysql and only delete from user - the rest mysql will do by itself. Refer here: mysqltutorial.org/mysql-on-delete-cascade \$\endgroup\$ – Your Common Sense May 19 '17 at 15:56
  • \$\begingroup\$ Will check that out as well, thanks for sharing! \$\endgroup\$ – user3304303 May 19 '17 at 20:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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