Help with optimizing this PHP code

[Teach]

Hello!

I hope you can help me please. I have a custom ticketing system which works great, and we have a Statistics page which is what this question is about.

To introduce the issue, we have two databases per ticket type. For this instance, I'm just including our main one, "support".

The main table is 'support', which contains all the ticket types, and then a 'supportreply' which contains all the responses from either staff or clients which include the ticket number to link it altogether. As I said, this all works fine.

The issue is as we're getting more and more tickets, our statistics page is running thousands of SQL queries to generate the stats. I know the code that's causing it, however, I need to know how to optimize it.

There is more than one ticket type, but I'm just including this as the others are the same... basically this code is going through each ticket (and its related replies in 'supportreply') to find out:

• Tickets that have had their first reply within 24 hours (and not)
• Average response time (from opening the ticket, to adding the first reply)
• Average variance time (from the ticket being submitted, to being opened)

 $over=0;
 $under=0;
 $rec2first=0;
 $variance=0;
 $total=0;
 $totvar=0;

 $tickets=mysql_query2("SELECT * FROM support");
 while ($ticket=mysql_fetch_array($tickets))
 {
   $firstreply=mysql_fetch_array(mysql_query2("SELECT * FROM supportreply WHERE id_ticket=".mysql_real_escape_string($ticket['id_ticket'])." AND user=1 ORDER BY dateadded"));
   $received = strtotime($ticket['dateadded']);
   $first = strtotime($firstreply['dateadded']);
   $opened = strtotime($ticket['dateopened']);

   $rec2first=$first-$received;
   $variance=$opened-$received;

   if ($rec2first>0)
   {
     $total=$total+$rec2first;
     $totvar=$totvar+$variance;
     if ($rec2first>86400)
       $over++;
     else
       $under++;
   }
 }

 $average_rec2first=$total/$tot_reply_db;
 $average_variance=$totvar/$tot_reply_db;

 $percent = $under/$tot_reply_db*100;
 if ($percent > '80') {
   echo "<span style=\"color: green;\">First reply in 24 hours: ".$under." / ".$tot_reply_db." (".floor($percent)."%)</span>\n";
 } else {
   echo "<span style=\"color: red;\">First reply in 24 hours: ".$under." / ".$tot_reply_db." (".floor($percent)."%)</span>\n";
 }

 $num_not_replied = $tot_reply_db-$under;
 $percent = $num_not_replied/$tot_reply_db*100;
 echo "<span>Over 24 hours: ".$over." / ".$tot_reply_db." (".floor($percent)."%)</span>\n";

 echo "<span>Response time:<br />".sec2hms($average_rec2first)."</span>\n";
 echo "<span>Variance time:<br />".sec2hms($average_variance)."</span>\n";

Any suggestions?

[dalecosp]

$tickets=mysql_query2("SELECT * FROM support"); 

Seems less than optimal.

Do some logic beforehand?

[Teach]

Hi there - thanks for the reply.

Yes, that's true! * was a silly idea. I therefore think I should be using this as this is the only credentials I use from each database.

This however won't lessen the amount of queries much though, right?

$tickets=mysql_query2("SELECT id_ticket, dateadded, dateopened FROM support");
$firstreply=mysql_fetch_array(mysql_query2("SELECT dateadded FROM supportreply WHERE id_ticket=".mysql_real_escape_string($ticket['id_ticket'])." AND user=1 ORDER BY dateadded"));

[djjjozsi]

test this query in phpmyadmin:

SELECT supportreply.dateadded , support.id_ticket, support.dateadded, support.dateopened  FROM support INNER JOIN supportreply  ON supportreply.id_ticket = support.id_ticket WHERE supportreply.user=1 ORDER BY dateadded

[dalecosp]

Well, lessening the amount of queries is probably a Good Thing(tm). OTOH, lessening the size of queries probably is as well. For example, let's say I have a db of 10,000 trouble tickets. Let's say only 650 of them are marked "open". Let's suppose that you can drawn conclusions from this:

$slow_query=mysql_query("select * from tickets");
$quicker_query=mysql_query("select * from tickets where status ="OPEN");

I'll apologize that I wasn't really giving specific help; it was just a general suggestion ... the first thing I saw was that query, and it just seemed rather inefficient. MySQL is pretty fast, but AFAIK (IANAE) it doesn't write 2MB of data to a socket as fast as it writes 500KB.

[dalecosp]

And I should add, looks like you gentlemen are on your way to figuring out something better (and specific) for this case. 🙂

[Teach]

Hello!

Firstly, thanks a lot for trying to help me, I really appreciate it.

Yeah, I can completely see your point with only selecting valid information, so I've taken that onboard 🙂

djjjozsi, wow, that is some complex SQL!!

I tried to run that query in phpmyadmin:

SELECT supportreply.dateadded , support.id_ticket, support.dateadded, support.dateopened  FROM support INNER JOIN supportreply  ON supportreply.id_ticket = support.id_ticket WHERE supportreply.user=1 ORDER BY dateadded

and it told me:

Error

SQL query: Documentation

SELECT supportreply.dateadded, support.id_ticket, support.dateadded, support.dateopened
FROM support
INNER JOIN supportreply ON supportreply.id_ticket = support.id_ticket
WHERE supportreply.user =1
ORDER BY dateadded
LIMIT 0 , 30

MySQL said: Documentation
#1052 - Column 'dateadded' in order clause is ambiguous 

Thanks again.

[djjjozsi]

Oh, i see.

supportreply table has a field: dateadded, and i did not determined which field is in used in the ORDER BY condition.

change the last dateadded into supportreply.dateadded

i don't want to force you to use my suggested query, but in several cases you can select the
information using one query.
Notice that: INNER JOIN will select the values from your two tables where the connected fields are in the two tables.
LEFT JOIN: this will select all the values from the table which is in the left side, and those values which is available in the right table.
RIGHT JOIN is the opposite of the LEFT JOIN

Read more about it: LEFT JOIN

[Teach]

Hi again, thanks for your help 😃

I had a read of that page, but it's all going a bit over my head. However, I updated the query to change that "dateadded" to "supportreply.dateadded" and the query successfully executed, and presented a list of tickets with:

dateadded   	 id_ticket   	 dateadded   	 dateopened

How could I use this to improve the both code and to lessen it's queries though? 😕

[djjjozsi]

one query means you don'T need to use the two while cycle to work with the results.

if you have fields in two tables with the same name (for example dateadded) always use aliases for one of that field..

you can use the mysql_fetch_assoc to fetch the results, the method is the same.

[Weedpacket]

$tickets=mysql_query2("SELECT * FROM support");
while ($ticket=mysql_fetch_array($tickets))
{
   $firstreply=mysql_fetch_array(mysql_query2("SELECT * FROM supportreply WHERE id_ticket=".mysql_real_escape_string($ticket['id_ticket'])." AND user=1 ORDER BY dateadded"));
   $received = strtotime($ticket['dateadded']);
   $first = strtotime($firstreply['dateadded']);
   $opened = strtotime($ticket['dateopened']);

   $rec2first=$first-$received;
   $variance=$opened-$received;

   if ($rec2first>0)
   {
     $total=$total+$rec2first;
     $totvar=$totvar+$variance;
     if ($rec2first>86400)
       $over++;
     else
       $under++;
   }
} 

As well as what has already been mentioned, I can see that the ORDER BY is unnecessary, since only summary data is used. In fact, the summary itself could be
done by the database

Select count(*) as over from supportreply r, support t where user=1 and r.id_ticket=t.id_ticket and r.dateadded > t.dateadded and TIMESTAMPDIFF(SECOND, r.dateadded, t.dateadded)>86400

Select count(*) as under from supportreply r, support t where user=1 and r.id_ticket=t.id_ticket and r.dateadded > t.dateadded and TIMESTAMPDIFF(SECOND, r.dateadded, t.dateadded)<=86400

Select
	SUM(TIMESTAMPDIFF(SECOND, r.dateadded,  t.dateadded)) as total,
	SUM(TIMESTAMPDIFF(SECOND, t.dateopened, t.dateadded)) as totvar,
From 
	supportreply r, support t
Where
	user=1
	and r.id_ticket=t.id_ticket
	and r.dateadded > t.dateadded

Three queries, no loop.

That's a first cut; no guarantee those queries work as-is (I don't have MySQL, so I haven't tested its TIMESTAMPDIFF function, for example); and I'm sure there are further improvements possible.

[Teach]

Hi Weedpacket, thanks for supplying that code for me!

Please bear with me whilst I try and understand how this fits into the original code I posted (or replaces it), and how I ultimately use it.

I see the "as x" part of the statements currently replaces: $over, $under, $total, $totvar

However, where would I implement the current $firstreply stuff too, and the $variance?

Just to resay, there are multiple instances of the above code (apart from those top 6 lines which introduce the variables) for different ticket types.

Hoping this is all making sense to you guys. I really am thankful for you helping me 🙂

[Weedpacket]

Teach wrote:

I see the "as x" part of the statements currently replaces: $over, $under, $total, $totvar

$sql = "Select count(*) as over ....";
$result = mysql_query($sql);
$row = mysql_fetch_array($result);
$over = $row['over'];

Teach wrote:

However, where would I implement the current $firstreply stuff too, and the $variance?

You don't; those only appear inside the loop, and as I said, the loop would be gone because the work it was doing had been shifted into the database.

Teach wrote:

Just to
resay, there are multiple instances of the above code

If you're doing the same thing over and over, then you're doing the computer's job.

[Teach]

Hi again! Thanks for the continued support.

I apologise to keep this going, but I'm not still not really getting it completely (although I understand what you mean by your last post).

I've so far based the below on your last post...

$sql = "SELECT COUNT(*) AS over FROM supportreply r, support t WHERE user=1 AND r.id_ticket=t.id_ticket AND r.dateadded > t.dateadded AND TIMESTAMPDIFF(SECOND, r.dateadded, t.dateadded)>86400";
$result = mysql_query($sql); 
$row = mysql_fetch_array($result); 
$over = $row['over'];

$sql = "SELECT COUNT(*) AS under FROM supportreply r, support t WHERE user=1 AND r.id_ticket=t.id_ticket AND r.dateadded > t.dateadded AND TIMESTAMPDIFF(SECOND, r.dateadded, t.dateadded)<=86400";
$result = mysql_query($sql); 
$row = mysql_fetch_array($result); 
$under = $row['under'];

$sql = "SELECT SUM(TIMESTAMPDIFF(SECOND, r.dateadded,  t.dateadded)) as total, SUM(TIMESTAMPDIFF(SECOND, t.dateopened, t.dateadded)) as totvar,	FROM supportreply r, support t WHERE user=1 AND r.id_ticket=t.id_ticket	AND r.dateadded > t.dateadded";
$result = mysql_query($sql); 
$row = mysql_fetch_array($result); 
$total = $row['total'];
$total = $row['totvar'];

This gets the $over, $under, $total and $totvar, but from my original code, would I no longer need the firstreply, variance etc?

If so, would I just simply add something like:

     if ($row['over']>86400) 
       $over++; 
     else 
       $under++; 

Thanks again and apologies for dragging this!

[Weedpacket]

Teach wrote:

This gets the $over, $under, $total and $totvar, but from my original code, would I no longer need the firstreply, variance etc?

No, those only appear inside the loop, and as I said, the loop would be gone because the work it was doing had been shifted into the database.

If so, would I just simply add something like:

No, because the counting is being done in the database, too.

[Teach]

Sorry to bump this, however, I'm retrying to get this working after getting busy with another project.

Although I'm no longer experiencing errors (which was happening before because apparantly "as over" is not valid, and there was a stray comma), I'm not getting the results I'm expecting or got correctly with the PHP solution above.

Here's the current code, followed by the problem:

 $over=0;
 $under=0;
 $total=0;
 $totvar=0;

 // Support
 $sql = "SELECT COUNT(*) AS `over` FROM supportreply r, support t WHERE user=1 AND r.id_ticket=t.id_ticket AND r.dateadded > t.dateadded AND TIMESTAMPDIFF(SECOND, r.dateadded, t.dateadded)>86400";
 $result = mysql_query($sql);
 $row = mysql_fetch_array($result);
 $over_support = $row['over'];

 $sql = "SELECT COUNT(*) AS `under` FROM supportreply r, support t WHERE user=1 AND r.id_ticket=t.id_ticket AND r.dateadded > t.dateadded AND TIMESTAMPDIFF(SECOND, r.dateadded, t.dateadded)<=86400";
 $result = mysql_query($sql);
 $row = mysql_fetch_array($result);
 $under_support = $row['under'];

 $sql = "SELECT SUM(TIMESTAMPDIFF(SECOND, r.dateadded, t.dateadded)) AS total, SUM(TIMESTAMPDIFF(SECOND, t.dateopened, t.dateadded)) AS totvar FROM supportreply r, support t WHERE user=1 AND r.id_ticket=t.id_ticket AND r.dateadded > t.dateadded";
 $result = mysql_query($sql);
 $row = mysql_fetch_array($result);
 $total_support = $row['total'];
 $totvar_support = $row['totvar'];

===Please remember there a few of these for different tickets, I'm then adding them all together with:===

 // Now add them all together...
 $over = $over_xxx + $over_xxx + (etc);
 $under = $under_xxx + $under_xxx + (etc);
 $total = $total_xxx + $total_xxx + (etc);
 $totvar = $totvar_xxx + $totvar_xxx + (etc);

The issue is that $under seems to be giving a figure that is even more than how many tickets there are in all the databases combined, so this is not a real figure.

I'm not sure if it's just checking whether the first reply to the ticket was within 24h, or all of them? This is what the PHP solution did, checking whether the first reply to the ticket was within 24 hours, if not, bump up $over, or if not bump up $under.

Because of this, I think this is also causing $total and $totvar to be totally out of whack (which they are in comparison to the PHP solution).

Thanks for any continued help you can provide.

[Teach]

Any ideas please guys? Really having problems wih this and everything I'm trying just doesn't seem to be happening right... 😕

Thanks.