Why is my PHP script (MySQL Query) so slow?

Question

I ran a script I made a couple of minutes ago and it still hasn't finished running.

Now it has, I got a 500 Internal Server Error.

EDIT--- The error is occuring on line 126 which is port of the while loop here:

while ($row == mysql_fetch_assoc($query)) {
            mysql_query("DELETE FROM `Likes` WHERE `accountID` = '$id'");
}

Please can you tell me what is causing my script to be so slow?

if ($param == "closeAccount") {
    $username = NULL;
    $password = NULL;
    $emailAdd = NULL;
    $id = NULL;
    if (isset($_GET['username'])) {
        $username = $_GET['username'];
    }
    else {
        exit("No username is set");
    }
    if (isset($_GET['password'])) {
        $password = md5($_GET['password']);
    }
    else {
        exit("No password is set");
    }
    if (isset($_GET['emailAddress'])) {
        $emailAdd = $_GET['emailAddress'];
    }
    else {
        exit("No email address is set");
    }
    $query = mysql_query("SELECT * FROM `Accounts` WHERE `Username` = '$username' AND `Password` = '$password' AND `Email Address` = '$emailAdd'");
    if (mysql_num_rows($query) < 1) {
        exit("Account doesn't exist");
    }
    while ($row = mysql_fetch_assoc($query)) {
        $id = $row["id"];
    }
    $query = NULL;
    $query = mysql_query("SELECT `id` FROM `Comments` WHERE `accountID` = '$id'");
    while ($row = mysql_fetch_assoc($query)) {
        mysql_query("DELETE FROM `Comments` WHERE `accountID` = '$id'") or die(mysql_error());
    }
    $query = NULL;
    $query = mysql_query("SELECT `id` FROM `Likes` WHERE `accountID` = '$id'");
    while ($row == mysql_fetch_assoc($query)) {
        mysql_query("DELETE FROM `Likes` WHERE `accountID` = '$id'");
    }
    $query = NULL;
    $query = mysql_query("SELECT `id` FROM `Posts` WHERE `accountID` = '$id'");
    while ($row == mysql_fetch_assoc($query)) {
        mysql_query("DELETE FROM `Posts` WHERE `accountID` = '$id'");
    }
    $query = NULL;
    $query = mysql_query("DELETE FROM `Accounts` WHERE `Username` = '$username' AND `Password` = '$password' AND `Email Address` = '$emailAdd'");
    exit("Closed");
}

you should avoid sql in while loops...actually in most loops — Paris Liakos, Jun 04 '11 at 23:50
you don't need single quotes around your table names in your queries, and yes, try to avoid queries in loops — Brett, Jun 04 '11 at 23:51
@Harigntka, see: http://stackoverflow.com/questions/332365/xkcd-sql-injection-please-explain — Johan, Jun 05 '11 at 15:47
@Johan I am talking about the -1, what was the point in that? — Harigntka, Jun 05 '11 at 15:52
The point is that your code is full of security holes: SQL-injection, hash with no salt. That's bad. — Johan, Jun 05 '11 at 15:58

score 6 · Accepted Answer · edited Jun 05 '11 at 11:32

6

It seems that you have a lot of redundancy. I dont have your direct database, so I could be slightly off on how you have your structure, but you may benefit from skipping the SELECT statements in the lower half.

Example:

$query = mysql_query("SELECT * FROM `Accounts` 
    WHERE `Username` = '$username' AND `Password` = '$password' 
    AND `Email Address` = '$emailAdd'");
if (mysql_num_rows($query) < 1) {
    exit("Account doesn't exist or wrong password entered");
}
$row = mysql_fetch_assoc($query);
$id = $row["id"]

$query = NULL;
mysql_query("DELETE FROM `Comments` WHERE `accountID` = '$id'");
mysql_query("DELETE FROM `Likes` WHERE `accountID` = '$id'");
mysql_query("DELETE FROM `Posts` WHERE `accountID` = '$id'");
mysql_query("DELETE FROM `Accounts` WHERE `id` = '$id'");

edited Jun 05 '11 at 11:32

Johan

74,508
24
191
319

answered Jun 04 '11 at 23:58

pyius

145
3

Works perfectly, and it does it almost immediately, thanks a lot :) – Harigntka Jun 05 '11 at 00:01
@Harigntka: It'll work even better if you add foreign key with `ON DELETE CASCADE` – a1ex07 Jun 05 '11 at 00:02
@a1ex07, what is a foreign key? – Harigntka Jun 05 '11 at 00:12
Foreign key is a way of creating relations between two tables. An entry (key) in table 1 points to an entry in table 2. Any change in table 2 is immediately reflected on table 1 without the need for the programmer to interfere. – N.B. Jun 05 '11 at 00:14
1

How could I set this up, it sounds like a much more efficient way of programming – Harigntka Jun 05 '11 at 00:15
It's a constraint used to enforce referential integrity in database. Take a look on http://dev.mysql.com/doc/refman/5.1/en/innodb-foreign-key-constraints.html . In mysql it works for INNODB engine only. – a1ex07 Jun 05 '11 at 00:17

score 2 · Answer 2 · answered Jun 04 '11 at 23:50

2

500 Internal Server Error should result in details logged in your web server error log on your server. Take a look in your web server's error log and go from there.

answered Jun 04 '11 at 23:50

Trott

66,479
23
173
212

score 1 · Answer 3 · answered Jun 04 '11 at 23:51

 $query = mysql_query("SELECT * FROM `Accounts` WHERE `Username` = '$username' AND `Password` = '$password' AND `Email Address` = '$emailAdd'");

Is likely to be failing.

Change them all to this

 $query = mysql_query("SELECT * FROM `Accounts` WHERE `Username` = '$username' AND `Password` = '$password' AND `Email Address` = '$emailAdd'") or die(mysql_error());

Edit :: Oh yea, there's a HUGE sql injection problem here. remember to do

$username = mysql_real_escape_string($_GET['usernmame']);

Unless you want injection to happen!

score 0 · Answer 4 · answered Jun 05 '11 at 00:00

You don't need to do a while here if you are suppose to get only one user. Just remove the while loop.

$query = mysql_query("SELECT * FROM `Accounts` WHERE `Username` = '$username' AND `Password` = '$password' AND `Email Address` = '$emailAdd'");
[...]
while ($row = mysql_fetch_assoc($query)) {
    $id = $row["id"];
}

Setting the $query variable to NULL is useless if you set it on the line after :

$query = NULL;
$query = mysql_query("SELECT `id` FROM `Comments` WHERE `accountID` = '$id'");

Here your SELECT queries are not used. They are useless, you can remove them.

$query = mysql_query("SELECT `id` FROM `Comments` WHERE `accountID` = '$id'");
while ($row = mysql_fetch_assoc($query)) {
    mysql_query("DELETE FROM `Comments` WHERE `accountID` = '$id'") or die(mysql_error());
}
$query = NULL;
$query = mysql_query("SELECT `id` FROM `Likes` WHERE `accountID` = '$id'");
while ($row == mysql_fetch_assoc($query)) {
    mysql_query("DELETE FROM `Likes` WHERE `accountID` = '$id'");
}
$query = NULL;
$query = mysql_query("SELECT `id` FROM `Posts` WHERE `accountID` = '$id'");
while ($row == mysql_fetch_assoc($query)) {
    mysql_query("DELETE FROM `Posts` WHERE `accountID` = '$id'");
}
$query = NULL;
$query = mysql_query("DELETE FROM `Accounts` WHERE `Username` = '$username' AND `Password` = '$password' AND `Email Address` = '$emailAdd'");

The while loop is useless in those cases, because you are always executing the SAME query. Replace that :

$query = mysql_query("SELECT `id` FROM `Comments` WHERE `accountID` = '$id'");
while ($row = mysql_fetch_assoc($query)) {
    mysql_query("DELETE FROM `Comments` WHERE `accountID` = '$id'") or die(mysql_error());
}

by that :

mysql_query("DELETE FROM `Comments` WHERE `accountID` = '$id'") or die(mysql_error());

score 0 · Answer 5 · answered Jun 05 '11 at 00:00

As noted, queries in a loop should be avoided at all cost.

Take that line you pointed out and do something like this below:

$ids_to_delete = array();
while ($row = mysql_fetch_assoc($query)) {
       $ids_to_delete[] = (int)  $row['id'];   
}
mysql_query("DELETE FROM `Likes` WHERE `accountID` IN(" . implode(',', $ids_to_delete) . ")");

Why is my PHP script (MySQL Query) so slow?

5 Answers5