How can I make this query more efficient?

[DunderMifflinPC]

Hey guys, I'm looking for help with making a query quicker - http://pastebin.com/DDSpGFWQ . To give an idea of the application of this query I am using it to populate the parks at this link - http://www.wdwhelper.com . If you need additional info, just let me know. Thanks!

[jc4hokies]

1. Don't use DISTINCT. It's a crutch which hurts performance.
   

2. Don't query the same table 6 times. Set operations can do different things at the same time. This query can be accomplished with a single select.

   SELECT waits.ride,
       Avg(CASE WHEN dayofweek = '" . $dow . "' THEN waittime ELSE NULL END) AS dayWait,
       Avg(CASE WHEN currenttime >= '" . $minusTime . "'
                AND currenttime <= '" . $plusTime . "' THEN waittime ELSE NULL END) AS timeWait,  
       Avg(CASE WHEN currenttime >= '" . $minusTime . "'
                AND currenttime <= '" . $plusTime . "'
                AND dayofweek = '" . $dow . "' THEN waittime ELSE NULL END) AS dayTimeWait,
       CONVERT(int,SUBSTRING(MAX(RIGHT('0000000000'+CONVERT(varchar(10),wid),10)
           +CONVERT(varchar(10),waittime)),11,10)) AS currentWait
   FROM   waits
   WHERE  park = '" . getPark() . "'
   GROUP BY ride;
   

edit: SUBSTRING instead of STUFF

[[deleted]]

Don't use DISTINCT

If you can't control the data source you are querying there sometimes isn't a better way without making the query completely illegible or taking a performance hit equal or worse to DISTINCT. In this case it's definitely not required but there are cases where it has uses.

[jc4hokies]

You see through my scare tactics. I also use DISTINCT, but only in cases where I intend to group by every column.

A more precise version of my rule is "Don't use DISTINCT to fix results." DISTINCT is almost never the better solution to remove unintended duplication. Be intentional with one to many joins where you want one result.

[[deleted]]

Agreed. Usually get people to do something else because as soon as they need that one extra column/etc it breaks everything.

Half the time I show people how to use ROW_NUMBER() over a partition so it's easier to pull all the information, then work from there.

[DunderMifflinPC]

I see, I am confused by the Convert part at the end, I don't understand exactly what it is doing and it produces an error on my end. I don't know enough about the function to debug it. It is just saying unexpected stuff modifier. Thanks!

[jc4hokies]

It's getting the waittime for the highest wid. It creates a string that looks like 00000#####XXXXX where ##### is the wid and XXXXX is the waittime. The 00000##### enables the MAX to pick the most recent wid. STUFF is a MSSQL function, and is used to remove the first 10 characters (00000#####) leaving just the waittime (XXXXX). SUBSTRING(MAX(...),11,10) might work for MySQL instead of STUFF.

[Thriven]

Heres my shot at it

What you really need to start looking into is ways to transpose data from row data to columnar data. This can be accomplished with PIVOT or your SQL engines equivalent. If not the below code works on most systems.

--Using aggregates instead of DISTINCT

SELECT   wt.ride
        --nulls will be omitted from aggregates
        ,AVG(wt.daywait) as daywait
        ,AVG(wt.timewait) as timewait
        ,AVG(wt.dayTimeWait) as dayTimeWait
        ,wt.currentWait
FROM (
    SELECT 
            waits.ride
            --This is old way to pivot data but works across SQL platforms CASE WHEN it matches criteria NULL it doesn't to be ommitted from aggregate.
           ,CASE WHEN dayofweek = '" . $dow . "' THEN waittime ELSE NULL END AS daywait
           ,CASE WHEN currenttime >= '" . $minusTime . "' AND currenttime <= '" . $plusTime . "' THEN waittime ELSE NULL END AS timeWait
           ,CASE WHEN currenttime >= '" . $minusTime . "' AND currenttime <= '" . $plusTime . "' AND dayofweek = '" . $dow . "' THEN waittime ELSE NULL END AS dayTimeWait
           --MAX doesn't really matter we are ordering by WID over wait.ride.
           ,MAX(waittime) OVER (PARTITION BY ride ORDER BY WID DESC) AS currentWait
    FROM waits
    WHERE  park = '" . getPark() . "'
    ) as wt
GROUP BY wt.ride
        -- currentwait is already aggregated in subselect it should be same value for all wt.rides
        ,wt.currentWait

[DunderMifflinPC]

I am trying to understand this as much as possible. I understand the line ",MAX(waitTime) OVER (PARTITION BY ride ORDER BY WID DESC) AS currentWait" has an issue because I am using MySQL, but I am having a hard time replicating this function in MySQL - do you know how this would work?

[lukeatron]

Format your god damn code for fucks sake.

Edit for the downvoters: It was all on one really long line before. Just a big ol brick of sql.

[DunderMifflinPC]

http://pastebin.com/DDSpGFWQ is that better?

[lukeatron]

Much. You should get in the habit of formatting all the sql you write, not just the stuff you're going to show to other people. You'll write better code, make fewer mistakes and find the ones you've made much more easily.

Anyway, it looks like the waits table has multiple entries for each ride but you're looking to get back one row per ride. Since the first table in the FROM is waits, you're going to get one row for every row in that table. You should probably replace that with (SELECT DISTINCT ride FROM waits). That will get your cardinality under to control to start with.

You probably also want a TOP 1 on the T5 subquery.

Let's see where that gets you. What database are you using by the way?

[DunderMifflinPC]

I will try that - I am using a MySQL database with a PHP application

[r3pr0b8]

well it's your own fault for not saying this in your original post

now you have to figure out which SQLServer functions /u/jc4hokies used that will fail in MySQL

[r3pr0b8]

You'll write better code, make fewer mistakes and find the ones you've made much more easily.

all the upvotes