Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This query currently takes approximately 250 seconds to run and I'd like to reduce it by as much as I can. I have tried using left joins instead of subqueries and it increases run time.

SELECT `items`.`id`,

ifnull((SELECT sum(`history`.`quantity`) FROM `history` where `history`.`itemID` = `items`.`id` and `history`.`date` > date_sub(now(), interval 7 DAY) group by `itemID`), '0') as `sold7`,
ifnull((SELECT round(`supply`) FROM `supply` WHERE `itemID` = `items`.`id` AND `date` > date_sub(now(), interval 7 DAY) group by `itemID`), '0') as `listed7`,

ifnull((SELECT sum(`history`.`quantity`) FROM `history` where `history`.`itemID` = `items`.`id` and `history`.`date` > date_sub(now(), interval 30 DAY) group by `itemID`), '0') as `sold30`,
ifnull((SELECT round(`supply`) FROM `supply` WHERE `itemID` = `items`.`id` AND `date` > date_sub(now(), interval 30 DAY) group by `itemID`), '0') as `listed30`,

ifnull((SELECT sum(`history`.`quantity`) FROM `history` where `history`.`itemID` = `items`.`id` and `history`.`date` > date_sub(now(), interval 60 DAY) group by `itemID`), '0') as `sold60`,
ifnull((SELECT round(`supply`) FROM `supply` WHERE `itemID` = `items`.`id` AND `date` > date_sub(now(), interval 60 DAY) group by `itemID`), '0') as `listed60`,

ifnull((SELECT sum(`history`.`quantity`) FROM `history` where `history`.`itemID` = `items`.`id` and `history`.`date` > date_sub(now(), interval 90 DAY) group by `itemID`), '0') as `sold90`,
ifnull((SELECT round(`supply`) FROM `supply` WHERE `itemID` = `items`.`id` AND `date` > date_sub(now(), interval 90 DAY) group by `itemID`), '0') as `listed90`,

ifnull((SELECT sum(`history`.`quantity`) FROM `history` where `history`.`itemID` = `items`.`id` and `history`.`date` > date_sub(now(), interval 180 DAY) group by `itemID`), '0') as `sold180`,
ifnull((SELECT round(`supply`) FROM `supply` WHERE `itemID` = `items`.`id` AND `date` > date_sub(now(), interval 180 DAY) group by `itemID`), '0') as `listed180`

FROM `items`

group by `items`.`id`
share|improve this question
    
What is the documented/expected result of this sub-select: SELECT round(`supply`) FROM `supply` WHERE `itemID` = `items`.`id` AND `date` > date_sub(now(), interval 7 DAY) group by `itemID` ? –  rolfl Jul 21 at 20:46
    
FYI, an SQL Fiddle I am playing with: sqlfiddle.com/#!9/d3913/1/0 –  rolfl Jul 21 at 20:50

2 Answers 2

Formatting

First, the way you wrote this makes it difficult to understand what you are trying to do. Here are a few points to improve it:

  • Use indentation and line breaks that illustrates your nesting. I provided an example below.
  • Use all caps for keywords (or all lower case, just be consistent).
  • Back ticks are not required unless you have a space or reserved operator in your code. It makes the code easier to follow without them.

Here is an example, this:

ifnull((SELECT sum(`history`.`quantity`) FROM `history` where `history`.`itemID` = `items`.`id` and `history`.`date` > date_sub(now(), interval 7 DAY) group by `itemID`), '0') as `sold7`,

Could be:

IFNULL(
    (SELECT SUM(history.quantity) 
    FROM history 
    WHERE history.itemID = items.id
    AND history.date > DATE_SUB(NOW(), INTERVAL 7 DAY) 
    GROUP BY itemID
    ), 0
) AS sold7,

Performance

The first problem I see is the excessive use of IFNULL() and subqueries within them. I'm not sure as to why you are replacing NULL with 0 as your aggregate functions will factor NULLs out anyways. But I suppose you have a reason to do this that I don't understand.

I have at least three recommendations:

  • If your dataset is very large, you may want to break it down into smaller steps using a temporary table.
  • You would likely want to use a LEFT JOIN instead of subqueries.
  • I would strongly suggest to create a stored procedure as this looks like the type of query that is run regularly. That way the execution plan will saved with your query.

Here you go:

DELIMITER $$
    CREATE OR REPLACE PROCEDURE proc_MyProcedure() AS -- pick a meaningful name
    BEGIN
    INSERT INTO TEMPORARY TABLE temp_MyTable -- pick a meaningful name
    	SELECT	items.id AS itemID,
    		history.quantity AS historyQty,
    		history.date AS historyDate,
    		supply.supply AS supplyQty,
    		supply.date AS supplyDate
    	FROM items 
    	LEFT JOIN supply ON items.id = supply.itemID
    	LEFT JOIN history ON items.id = history.itemID
    	WHERE	history.date > DATE_SUB(NOW(), INTERVAL 180 DAY)
    	OR	supply.date  > DATE_SUB(NOW(), INTERVAL 180 DAY)
    ;
    -- here we set nulls to 0
    UPDATE temp_MyTable
    SET historyQty = 0
    WHERE historyQty IS NULL;
    UPDATE temp_MyTable
    SET supplyQty = 0
    WHERE supplyQty IS NULL;
    -- finally we aggregate on the temp table
    SELECT 	(SELECT SUM(historyQty) FROM temp_Mytable WHERE history.date > DATE_SUB(NOW(), INTERVAL 7 DAY)) AS sold7,
    	(SELECT ROUND(supply) FROM temp_MyTable WHERE supplyDate > DATE_SUB(NOW(), INTERVAL 7 DAY)) AS listed7,
    -- etc.
    FROM temp_MyTable
    GROUP BY itemID;
    END$$
DELIMITER ;

Then just CALL proc_MyProcedure(). You will notice the first time takes longer, but any subsequent calls should be much faster.

share|improve this answer

Using a case statement, instead of the subselects, allows you to do logic on the results of a join, instead of the conditions of a join. But, first let's look at potential bugs in your code....

Bugs:

  1. You group by columns when you do not need to:

    SELECT sum(`history`.`quantity`) FROM `history` where `history`.`itemID` = `items`.`id` and `history`.`date` > date_sub(now(), interval 7 DAY) group by `itemID`
    

    In the above subselect, SUM() is an aggregation function. It aggregates all the values in a given group. You group by itemID, but, you also have the nested join condition where \history`.`itemID` = `items`.`id`` .... which means your subselect will only return values for a single itemID anyway. There is no need to group the subselect.

  2. MySQL has this feature which I call a bug, and that is that it allows you to group by columns that are not in the select, and not group by non-aggregated columns that are in the select: The 'feature' documentation here . The most interesting part is this:

    The server is free to choose any value from each group, so unless they are the same, the values chosen are indeterminate. Furthermore, the selection of values from each group cannot be influenced by adding an ORDER BY clause.

    The reason this is significant for you, is that you have the following sub-selects:

    SELECT round(`supply`) FROM `supply` WHERE `itemID` = `items`.`id` AND `date` > date_sub(now(), interval 7 DAY) group by `itemID`
    

    ROUND() is NOT an aggregation function, so it does not work the same way as SUM(), MAX(), etc. So, in the context of the MySQL feature, and the group by you have above, the column supply is a victim of the the values chosen are indeterminate part of the 'not-grouped-by' feature of MySQL. In other words, if you have multiple supply records for your itemID, you will get 'random' results for the round(supply).

Performance

The biggest performancec issue here is the multiple subselects. These can all be neatly removed by using a trick of the wonderful case syntax. My SQL history goes back to a time when case did not exist, and, it makes the world of difference. Use it.

Since your round() subselects are buggy, and I don't know what you expect the real results to be, I'll pretend you want to round the 'average' value of those columns. You will need to modify those columns to suit your needs:

SELECT `items`.`id`,
    IfNull(sold7, 0) as sold7,
    IfNull(listed7, 0) as listed7,
    IfNull(sold30, 0) as sold30,
    IfNull(listed30, 0) as listed30,
    IfNull(sold60, 0) as sold60,
    IfNull(listed60, 0) as listed60,
    IfNull(sold90, 0) as sold90,
    IfNull(listed90, 0) as listed90,
    IfNull(sold180, 0) as sold180,
    IfNull(listed180, 0) as listed180,

    1 as `dummy`

FROM `items`
     left join (select
        `itemID`,
        sum((case when `date` > date_sub(now(), interval 7  DAY) then `quantity` else 0 end)) as `sold7`,
        sum((case when `date` > date_sub(now(), interval 30 DAY) then `quantity` else 0 end)) as `sold30`,
        sum((case when `date` > date_sub(now(), interval 60 DAY) then `quantity` else 0 end)) as `sold60`,
        sum((case when `date` > date_sub(now(), interval 90 DAY) then `quantity` else 0 end)) as `sold90`,
        sum((case when `date` > date_sub(now(), interval 180 DAY) then `quantity` else 0 end)) as `sold180`
      from `history`
      where `date` >= date_sub(now(), interval 180 DAY)
      group by `itemID`
      ) as `hist` on `hist`.`itemID` = `items`.`ID`
     left join (select
        itemID,
        round(avg((case when `date`  > date_sub(now(), interval 7  DAY) then `supply` else 0 end))) as `listed7`,
        round(avg((case when `date`  > date_sub(now(), interval 30 DAY) then `supply` else 0 end))) as `listed30`,
        round(avg((case when `date`  > date_sub(now(), interval 60 DAY) then `supply` else 0 end))) as `listed60`,
        round(avg((case when `date`  > date_sub(now(), interval 90 DAY) then `supply` else 0 end))) as `listed90`,
        round(avg((case when `date`  > date_sub(now(), interval 180 DAY) then `supply` else 0 end))) as `listed180`
       from supply
       where `date` >= date_sub(now(), interval 180 DAY)
       group by itemID
     ) as `supp` on `supp`.`itemID` = `items`.`ID`

The above query will do no direct joins on either the supply or the history table, and will just do a join on the aggregated results. There may be many items, but it should be a lot faster than the many subselects you currently have. The case-statement makes it work.

Even though the two aggregating statements are represented as subselects (in from clauses), they have no references back to the outer items table so there is no need for the database to do the join to the outer items until after the aggregation.

I have put this together as an SQLFiddle

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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