[Resolved] Simplifying a function

[discostudio]

Hi everyone.

I am using the following function, and I would like to simplify it.

// This function deletes old trolleys when they are over a year old.
function DelOldTrolley() {
$erasedate = date("Y-m-d", mktime(0,0,0,date("m"),date("d"),date("Y")));
mysql_query("DELETE FROM trolley WHERE Expire < '$erasedate' AND active = '1'") or die(mysql_error()); // old query, no longer used...
// Purge TrolleyContent
$ql2 = mysql_query("SELECT * FROM trolleycontent") or die(mysql_error());
while($ql3=mysql_fetch_array($ql2)) {
//echo $ql3[TrolleyId]." Selected!<br>";
$q83 = mysql_query("SELECT * FROM trolley WHERE TrolleyId = '".$ql3[TrolleyId]."'") or die(mysql_error());
$q84 = mysql_num_rows($q83);

if ($q84==0) { 
mysql_query("DELETE FROM trolleycontent WHERE TrolleyId = '".$ql3[TrolleyId]."'") or die(mysql_error());
//echo $ql3[TrolleyId]." Deleted!<br>";
}
}

I would like to simplify it and hopefully condense it into one single query.

[discostudio]

So to just explain it a little better i have to do the following:

1. Query the trolley rows to find any that are still active=1 and are more than 2 days old.
   (And leave the row alone if active =0)

{
2. Then select each trolley content rows one by one and delete them.
}

3. Finally delete the trolley row conecerned.

Hopefully all in one query.

[laserlight]

You could try something along these lines:

//function to delete old trolleys where they are over a year old
function DelOldTrolley() {
	$erasedate = date("Y-m-d", mktime(0,0,0,date("m"),date("d"),date("Y")));
	$query = "SELECT TrolleyId
		FROM trolley
		WHERE Expire < '$erasedate' AND active='1'";
	$result = mysql_query($query)
		OR die(mysql_error());
	//loop through trolleys to be deleted
	while ($row = mysql_fetch_array($result)) {
		$tid = $row['TrolleyId'];
		//purge trolleycontent for this trolley
		mysql_query("DELETE FROM trolleycontent WHERE TrolleyId='$tid'")
			OR die(mysql_error());
		//delete this trolley
		mysql_query("DELETE FROM trolley WHERE TrolleyId='$tid'")
			OR die(mysql_error());
	}
}

[Roger_Ramjet]

You can do this sort of thing with 1 query:
You can implement referential integrity and cascading deletes . For this you need to use InnoDB type tables. If you are using MySQL version 4.
You can also do multi-table deletes using a join with ver 4.

"DELETE trolley, trolleycontent FROM trolley, trolleycontent USING (TrolleyId) WHERE trolley.Expire < '$erasedate' AND trolley.active='1'";

should just about do it. Of course, if you are using an older version then multi-table deletes are not supported.

(good reason to upgrade maybe)

[jbx]

There were some errors in your code, such as the $erasedate would actually equal today and not two days ago. But we can tidy that up by performing the calculation in the MySQL instead of the PHP. I'm not sure that this can be done in one query using V3.23.58 which is what I'm running, but if anyone can do this in fewer queries I would love to know how. Still, this function will work fine and allows you to dynamically adjust the number of days for expiry. Hope it helps...

function purgeTrolley($days) {
	// Find all entries in the trolley which are active and more than '$days' days old...
	$result = mysql_query("SELECT TrolleyId FROM trolley WHERE active = '1' AND expire < DATE_SUB(CURDATE(), INTERVAL $days DAY)")
	or die("Query Error: ".mysql_error());

// Save some effort if there aren't any results
if(mysql_num_rows($result) > 1) {

	// Loop through the results
	while($r = mysql_fetch_assoc($result)) {

		// Delete the row in `trolley`
		mysql_query("DELETE FROM trolley WHERE TrolleyId = '$r[TrolleyId]'")
		or die("Query Error: ".mysql_error());

		// And any rows in the `trolleycontent` table
		mysql_query("DELETE FROM trolleycontent WHERE TrolleyId = '$r[TrolleyId]'")
		or die("Query Error: ".mysql_error());
	}
}
}

// Call the function, specifying the number of days for expiry
purgeTrolley(2);

[Roger_Ramjet]

Nope, has to be 4, same for doing it with a subquery.

I will just reduce your function to the minimum number of queries though.

function purgeTrolley($days) {
    // Find all entries in the trolley which are active and more than '$days' days old...
    $result = mysql_query("SELECT TrolleyId FROM trolley WHERE active = '1' AND expire < DATE_SUB(CURDATE(), INTERVAL $days DAY)")
    or die("Query Error: ".mysql_error());

// Save some effort if there aren't any results
if(mysql_num_rows($result) > 1) {

      // Delete the trollies in `trolley`
      mysql_query("DELETE FROM trolley WHERE  active = '1' AND expire < DATE_SUB(CURDATE(), INTERVAL $days DAY)")
      or die("Query Error: ".mysql_error());

      // Loop through the results
    while($r = mysql_fetch_assoc($result)) {

        // And any rows in the `trolleycontent` table
        mysql_query("DELETE FROM trolleycontent WHERE TrolleyId = '$r[TrolleyId]'")
        or die("Query Error: ".mysql_error());
    }
}
}

// Call the function, specifying the number of days for expiry
purgeTrolley(2);

[discostudio]

Thanks JBX

That was an unbelievable reply.

I have just implemented your purgeTrolleys function and it works a treat, all i have to do is state that i would like to delete any trolleys that are still active and over $days old.

Brilliant.

Thanks Jon.

[jbx]

No problem craig, although I would use Rogers version
as that is the correct place to put the second query,
otherwise it's being run pointlessly on each loop

(Thanks Roger, it's one of those mistakes I often make
and have to go back and correct in my own code. I'm
always putting the loops too early! 😉 )