views:

359

answers:

5

Good afternoon all. I'm going to post the stored procedure in it's entire glory. Feel free to rip it to shreds. The author won't mind.

DECLARE @itemTypeID INT
SELECT @itemTypeID=ItemTypeID FROM dbo.ItemTypes WHERE ItemTypeName = 'Advert'

BEGIN
 SELECT a.Active,
   a.ParentClass,
   a.Classification,
   a.Variant,
   FV."Full Views",
   PV."Print Views",
   EE."Email Enquiries",
   a.ItemRef,
   a.SiteID
 FROM 
 (
 SELECT DISTINCT i.ItemID,
   i.ItemRef,
   i.SiteID,
   i.ParentClass,
   i.Classification,
   i.Summary AS "Variant",       
   i.Active
 FROM Items i
 JOIN Actions a
 ON a.ItemID = i.ItemID
 JOIN ActionTypes at 
 ON a.ActionTypeID = at.ActionTypeID
 WHERE i.ItemTypeID = 1
 AND a.DateAndTime BETWEEN @startDate AND @endDate
 AND at.ActionTypeName IN ('Full view', 'Print view', 'Email enquiry')
 AND ((@siteID = -1) OR (i.SiteID = @siteID))
 AND ((@parentClass = '%') OR (i.ParentClass = @parentClass))
 AND ((@class = '%') OR (i.classification = @class))
 ) a LEFT JOIN
 (
 SELECT i.ItemID, 
   COUNT(*) AS "Full Views"
 FROM CustomerSites cs JOIN Items i
 ON cs.SiteID = i.SiteID
 JOIN Actions a
 ON a.ItemID = i.ItemID
 JOIN ActionTypes at 
 ON a.ActionTypeID = at.ActionTypeID
 JOIN Sites s
 ON cs.SiteID = s.SiteID
 WHERE a.DateAndTime BETWEEN @startDate AND @endDate
 AND i.ItemTypeID = @itemTypeID
 AND at.ActionTypeName = 'Full view'
 AND ((@customerID IS NULL) OR (cs.CustomerID = @customerID))
 AND ((@siteID = -1) OR (cs.SiteID = @siteID))
 AND ((@parentClass = '%') OR (i.ParentClass = @parentClass))
 AND ((@class = '%') OR (i.classification = @class))
 GROUP BY
   i.ItemID
 ) FV 
 ON a.ItemID = FV.ItemID
 LEFT JOIN
 (
 SELECT i.ItemID,
   COUNT(*) AS "Print Views"
 FROM CustomerSites cs JOIN Items i
 ON cs.SiteID = i.SiteID
 JOIN Actions a
 ON a.ItemID = i.ItemID
 JOIN ActionTypes at 
 ON a.ActionTypeID = at.ActionTypeID
 JOIN Sites s
 ON cs.SiteID = s.SiteID
 WHERE a.DateAndTime BETWEEN @startDate AND @endDate
 AND i.ItemTypeID = @itemTypeID
 AND at.ActionTypeName = 'Print view'
 AND ((@customerID IS NULL) OR (cs.CustomerID = @customerID))
 AND ((@siteID = -1) OR (cs.SiteID = @siteID))
 AND ((@parentClass = '%') OR (i.ParentClass = @parentClass))
 AND ((@class = '%') OR (i.classification = @class))
 GROUP BY
   i.ItemID
 ) PV 
 ON a.ItemID = PV.ItemID
 LEFT JOIN
 (
 SELECT i.ItemID,
   COUNT(*) AS "Email Enquiries"
 FROM CustomerSites cs JOIN Items i
 ON cs.SiteID = i.SiteID
 JOIN Actions a
 ON a.ItemID = i.ItemID
 JOIN ActionTypes at 
 ON a.ActionTypeID = at.ActionTypeID
 JOIN Sites s
 ON cs.SiteID = s.SiteID
 WHERE a.DateAndTime BETWEEN @startDate AND @endDate
 AND i.ItemTypeID = @itemTypeID
 AND at.ActionTypeName = 'Email enquiry'
 AND ((@customerID IS NULL) OR (cs.CustomerID = @customerID))
 AND ((@siteID = -1) OR (cs.SiteID = @siteID))
 AND ((@parentClass = '%') OR (i.ParentClass = @parentClass))
 AND ((@class = '%') OR (i.classification = @class))
 GROUP BY
   i.ItemID
 ) EE
 ON a.ItemID = EE.ItemID
 UNION
 SELECT '0','','','','','','','',''

Now ultimately all this does is return some records and the number of times a particular action has occured against them.

A small subset would look like.

Item Description Views Printed Emails

Item1 Desc1 12 NULL 1
Item2 Desc2 NULL NULL 3
Item3 Desc3 5 6 2

Hopefully you can see what's going on.

I want a list of items who have had actions against them for a particular date range for a particular customer for a particular site and the query should be filterable on parent class and classification. Nice

The first select returns all distinct items that fall within the selection criteria.

The other 3 queries all simply returning counts of 1 type of action against each item. The query is pants slow even against a small amount of data. This is never going to go live it just won't work.

Hopefully you can see the error of the 'authors' ways and correct him/her.

+4  A: 

here is the original query with my formatting style:

SELECT  
    a.Active
        ,a.ParentClass
        ,a.Classification
        ,a.Variant
        ,FV."Full Views"
        ,PV."Print Views"
        ,EE."Email Enquiries"
        ,a.ItemRef
        ,a.SiteID
    FROM (SELECT DISTINCT
              i.ItemID,
                  ,i.ItemRef
                  ,i.SiteID
                  ,i.ParentClass
                  ,i.Classification
                  ,i.Summary AS "Variant"
                  ,i.Active
              FROM Items              i
                  JOIN Actions        a ON a.ItemID = i.ItemID
                  JOIN ActionTypes   at ON a.ActionTypeID = at.ActionTypeID
              WHERE i.ItemTypeID = 1
                  AND a.DateAndTime BETWEEN @startDate AND @endDate
                  AND at.ActionTypeName IN ('Full view', 'Print view', 'Email enquiry')
                  AND ((@siteID = -1) OR (i.SiteID = @siteID))
                  AND ((@parentClass = '%') OR (i.ParentClass = @parentClass))
                  AND ((@class = '%') OR (i.classification = @class))
        ) a
        LEFT JOIN (SELECT  
                       i.ItemID
                           ,COUNT(*) AS "Full Views"
                       FROM CustomerSites          cs
                           JOIN Items               i ON cs.SiteID = i.SiteID
                           JOIN Actions             a ON a.ItemID = i.ItemID
                           JOIN ActionTypes        at ON a.ActionTypeID = at.ActionTypeID
                           JOIN Sites               s ON cs.SiteID = s.SiteID
                      WHERE a.DateAndTime BETWEEN @startDate AND @endDate
                          AND i.ItemTypeID = @itemTypeID
                          AND at.ActionTypeName = 'Full view'
                          AND ((@customerID IS NULL) OR (cs.CustomerID = @customerID))
                          AND ((@siteID = -1) OR (cs.SiteID = @siteID))
                          AND ((@parentClass = '%') OR (i.ParentClass = @parentClass))
                          AND ((@class = '%') OR (i.classification = @class))
                      GROUP BY i.ItemID
                  ) FV ON a.ItemID = FV.ItemID
        LEFT JOIN (SELECT
                       i.ItemID
                           ,COUNT(*) AS "Print Views"
                       FROM CustomerSites      cs 
                           JOIN Items           i ON cs.SiteID = i.SiteID
                           JOIN Actions         a ON a.ItemID = i.ItemID
                           JOIN ActionTypes    at ON a.ActionTypeID = at.ActionTypeID
                           JOIN Sites           s ON cs.SiteID = s.SiteID
                       WHERE a.DateAndTime BETWEEN @startDate AND @endDate
                           AND i.ItemTypeID = @itemTypeID
                           AND at.ActionTypeName = 'Print view'
                           AND ((@customerID IS NULL) OR (cs.CustomerID = @customerID))
                           AND ((@siteID = -1) OR (cs.SiteID = @siteID))
                           AND ((@parentClass = '%') OR (i.ParentClass = @parentClass))
                           AND ((@class = '%') OR (i.classification = @class))
                       GROUP BY i.ItemID
                  ) PV ON a.ItemID = PV.ItemID
        LEFT JOIN (SELECT
                       i.ItemID
                           ,COUNT(*) AS "Email Enquiries"
                       FROM CustomerSites   cs
                           JOIN Items        i ON cs.SiteID = i.SiteID
                           JOIN Actions      a ON a.ItemID = i.ItemID
                           JOIN ActionTypes at ON a.ActionTypeID = at.ActionTypeID
                           JOIN Sites        s ON cs.SiteID = s.SiteID
                       WHERE a.DateAndTime BETWEEN @startDate AND @endDate
                           AND i.ItemTypeID = @itemTypeID
                           AND at.ActionTypeName = 'Email enquiry'
                           AND ((@customerID IS NULL) OR (cs.CustomerID = @customerID))
                           AND ((@siteID = -1) OR (cs.SiteID = @siteID))
                           AND ((@parentClass = '%') OR (i.ParentClass = @parentClass))
                           AND ((@class = '%') OR (i.classification = @class))
                       GROUP BY i.ItemID
                  ) EE ON a.ItemID = EE.ItemID
UNION
SELECT '0','','','','','','','',''

this should help a little:

;WITH CustomerSitesCounts AS
(
SELECT
    at.ActionTypeName
        ,i.ItemID
        ,COUNT(*) AS "Print Views"
    FROM CustomerSites      cs 
        JOIN Items           i ON cs.SiteID = i.SiteID
        JOIN Actions         a ON a.ItemID = i.ItemID
        JOIN ActionTypes    at ON a.ActionTypeID = at.ActionTypeID
        JOIN Sites           s ON cs.SiteID = s.SiteID
    WHERE a.DateAndTime BETWEEN @startDate AND @endDate
        AND i.ItemTypeID = @itemTypeID
        AND at.ActionTypeName IN ( 'Print view','Full view','Email enquiry')
        AND ((@customerID IS NULL) OR (cs.CustomerID = @customerID))
        AND ((@siteID = -1) OR (cs.SiteID = @siteID))
        AND ((@parentClass = '%') OR (i.ParentClass = @parentClass))
        AND ((@class = '%') OR (i.classification = @class))
    GROUP BY at.ActionTypeName,i.ItemID
)
SELECT  
    a.Active
        ,a.ParentClass
        ,a.Classification
        ,a.Variant
        ,FV."Full Views"
        ,PV."Print Views"
        ,EE."Email Enquiries"
        ,a.ItemRef
        ,a.SiteID
    FROM (SELECT DISTINCT
              i.ItemID,
                  ,i.ItemRef
                  ,i.SiteID
                  ,i.ParentClass
                  ,i.Classification
                  ,i.Summary AS "Variant"
                  ,i.Active
              FROM Items              i
                  JOIN Actions        a ON a.ItemID = i.ItemID
                  JOIN ActionTypes   at ON a.ActionTypeID = at.ActionTypeID
              WHERE i.ItemTypeID = 1
                  AND a.DateAndTime BETWEEN @startDate AND @endDate
                  AND at.ActionTypeName IN ('Full view', 'Print view', 'Email enquiry')
                  AND ((@siteID = -1) OR (i.SiteID = @siteID))
                  AND ((@parentClass = '%') OR (i.ParentClass = @parentClass))
                  AND ((@class = '%') OR (i.classification = @class))
        ) a
        LEFT JOIN CustomerSitesCounts FV ON a.ItemID = FV.ItemID AND FV.ActionTypeName='Full view'
        LEFT JOIN CustomerSitesCounts PV ON a.ItemID = PV.ItemID AND PV.ActionTypeName='Print view'
        LEFT JOIN CustomerSitesCounts EE ON a.ItemID = EE.ItemID AND EE.ActionTypeName='Email enquiry'
UNION
SELECT '0','','','','','','','',''
KM
Looks good, KM... I would extract that Item/Actions/ActionTypes portion into a base CTE that the other CTE and the main query share, further eliminating the duplicate code (won't necessarily impact the the query plan, just for display).
richardtallent
@KM: Are you putting the commas at the beginning of lines purely for source control (i.e. adding a column changes 1 line instead of 2)?
Paul Williams
+1  A: 

First of all, you can avoid having a copy of 3 absolutely the same sub-queries buy having a generic one and use WITH statement so that you can reuse it.

Then, why doing a subselect when not really required.

Then again, remove some joins (and therefore DISTINCTs) that you do not need.

And you get something along these lines (not tested, naturally):

DECLARE @itemTypeID INT
SELECT @itemTypeID=ItemTypeID FROM dbo.ItemTypes WHERE ItemTypeName = 'Advert'

BEGIN
    WITH ItemTypeSummary
    AS (SELECT  i.ItemID,
                at.ActionTypeName,
                COUNT(*) AS CNT
        FROM    Items i
        JOIN    Actions a
            ON  a.ItemID = i.ItemID
        JOIN    ActionTypes at 
            ON  a.ActionTypeID = at.ActionTypeID
            AND at.ActionTypeName IN ('Full view', 'Print view', 'Email enquiry')
        WHERE   a.DateAndTime BETWEEN @startDate AND @endDate
            --// not sure you need those below as they are all part of Items filter anyways in the main query
            /*
            AND i.ItemTypeID = @itemTypeID
            AND ((@customerID IS NULL) OR (cs.CustomerID = @customerID))
            AND ((@siteID = -1) OR (cs.SiteID = @siteID))
            AND ((@parentClass = '%') OR (i.ParentClass = @parentClass))
            AND ((@class = '%') OR (i.classification = @class))
            */
        GROUP BY    i.ItemID,
                    at.ActionTypeName
    )

    SELECT DISTINCT i.ItemID,
                    i.ItemRef,
                    i.SiteID,
                    i.ParentClass,
                    i.Classification,
                    i.Summary AS "Variant",
                    i.Active,
                    FV.CNT AS "Full views",
                    PV.CNT AS "Print views",
                    EE.CNT AS "Email enquiries"
    FROM        Items i
    JOIN        CustomerSites cs
            ON  cs.SiteID = i.SiteID
    LEFT JOIN   ItemTypeSummary FV
            ON  i.ItemID = FV.ItemID
            AND FV.ActionTypeName = 'Full view'
    LEFT JOIN   ItemTypeSummary PV
            ON  i.ItemID = PV.ItemID
            AND PV.ActionTypeName = 'Print view'
    LEFT JOIN   ItemTypeSummary EE
            ON  i.ItemID = EE.ItemID
            AND EE.ActionTypeName = 'Email enquiry'

    WHERE   i.ItemTypeID = @itemTypeID
        AND ((@customerID IS NULL) OR (cs.CustomerID = @customerID))
        AND ((@siteID = -1) OR (i.SiteID = @siteID))
        AND ((@parentClass = '%') OR (i.ParentClass = @parentClass))
        AND ((@class = '%') OR (i.classification = @class))
END

Also keep in mind that doing a trick of filter ALL or specific item that you have with those OR statements is not really cool if you can avoid them, because SQLServer will not be able to generate optimal execution plan.

van
+1 for the OR comment; breaking this up into a UNION of multiple results without the OR *might* perform better
Paul Williams
Well this certainly is different. The reason the filters were applied to the very first select was to reduce the number of returned items that I was joining to. The above query however returns items that have had no action against them.The above joins to every item in the item table that a particular customer has ever had and not only those that had an action in the time period. I'll see if i can tweak your example a bit thanks.
Robert
in ItemTypeSummary, your select columns do not match you group by columns, which will result in an error
KM
@Robert, that is why I didn't refactor to much in my version. It is hard for my to tell what is going on in your tables.
KM
-- @KM: thanks, fixed. -- @Robert: you may need some filters, and do not need others. In general the SQL server should do a good job not computing the summary for the items types you do not need. The date range filter definitely must be in the ItemTypeSummary subview. Others were just duplications IMHO. If you could post some sample data and expected results, we could check the queries. Given the information we have, we can only suggests the "ways" to improve the query, and not provide the final result as such. Cheers.
van
+1  A: 

Another option is to setup either table variable or a temporary #Table to hold the results. Add a unique constraint on the ItemID.

Break apart the sub-equeries into separate steps. First INSERT the records you want to count into this table. Run separate UPDATEs on the data for each view type-- UPDATE the Print View count, UPDATE the Full View count, UPDATE the Email Enquiry count. Then return the results. If necessary, split apart the OR conditions into separate queries.

This approach runs through your data several times, but it avoids LEFT JOINs and sub-queries that aren't indexed.

In our app, having multiple steps seems to perform better than one very complex query. Your results may vary.

Paul Williams
Unfortunately the temp tables didn't improve the performance to a great amount but it was an improvement. Going for the dynamic SQL option.
Robert
+3  A: 

Problems:

  1. "catch-all" type queries cannot be optimized. Solution: parametrized dynamic SQL.
  2. UNION forces sorting. Solution: use UNION ALL instead (unless you really need the implicit DISTINCT that it forces).

This should be the fastest solution:

DECLARE @sql AS VARCHAR(MAX)

SELECT @sql = '
;WITH cteCommon as (
    SELECT 
     i.ItemID,
     ,i.ItemRef
     ,i.SiteID
     ,i.ParentClass
     ,i.Classification
     ,i.Summary
     ,i.Active
     ,at.ActionTypeName
    FROM Items              i
     JOIN Actions        a ON a.ItemID = i.ItemID
     JOIN ActionTypes   at ON a.ActionTypeID = at.ActionTypeID
    WHERE at.ActionTypeName IN (''Full view'', ''Print view'', ''Email enquiry'')
     --NOTE: if you max-out this date range, then you mant want to exclude it also, RBarryYoung
     AND a.DateAndTime BETWEEN @startDate AND @endDate 
     ' 
     + CASE @siteid WHEN -1 THEN '' ELSE 'AND (i.SiteID = @siteID) 
     ' END
     + CASE @parentClass WHEN '%' THEN '' ELSE 'AND (i.ParentClass = @parentClass)
     ' END
     + CASE @class WHEN '%' THEN '' ELSE 'AND (i.classification = @class)
     ' END
     + '
)
, cteA as (
    SELECT DISTINCT
       i.ItemID,
       ,i.ItemRef
       ,i.SiteID
       ,i.ParentClass
       ,i.Classification
       ,i.Summary AS "Variant"
       ,i.Active
      FROM cteA as i
      WHERE i.ItemTypeID = 1
)
, cteCountViews AS (
    SELECT  
     i.ItemID
     ,i.ActionType
     ,COUNT(*) AS "ViewCount"
    FROM cteCommon i
     JOIN CustomerSites       cs ON cs.SiteID = i.SiteID
     JOIN Sites               s ON cs.SiteID = s.SiteID
    WHERE i.ItemTypeID = @itemTypeID
     '
     + CASE WHEN @customerid IS NULL THEN '' ELSE '(cs.CustomerID = @customerID)' END
     + '
    GROUP BY i.ItemID
     ,i.ActionType
    )
SELECT  
    a.Active
    ,a.ParentClass
    ,a.Classification
    ,a.Variant
    ,FV."Full Views"
    ,PV."Print Views"
    ,EE."Email Enquiries"
    ,a.ItemRef
    ,a.SiteID
FROM cteA AS a
LEFT JOIN (
     SELECT i.ItemID, ViewCount AS "Full Views"
     FROM cteCountViews i
     WHERE i.ActionTypeName = ''Full view''
    ) FV ON a.ItemID = FV.ItemID
LEFT JOIN (
     SELECT i.ItemID, ViewCount AS "Print Views"
     FROM cteCountViews i
     WHERE i.ActionTypeName = ''Print view''
    ) PV ON a.ItemID = PV.ItemID
LEFT JOIN (
     SELECT i.ItemID, ViewCount AS "Email Enquiries"
     FROM cteCountViews i
     WHERE i.ActionTypeName = ''Email enquiry''
      ) EE ON a.ItemID = EE.ItemID
UNION ALL
SELECT ''0'','''','''','''','''','''','''','''',''''
'
EXEC sp_ExecuteSQL @sql
    ,'@startdate DATETIME,@enddate DATETIME,@siteid INT,@parentclass VARCHAR(MAX),@class VARCHAR(MAX),@itemtypeid INT,@customerid INT'
    , @startdate, @enddate, @siteid, @parentclass, @class, @itemtypeid, @customerid

Note: your use of wildcards on some of thes (class, sites, etc.) with JOINs is likely to cause cross-multiplication of your source rows and enormous result sets.

RBarryYoung
Dynamic SQL here we go. I do want to keep the union as it forces the blank row to be first. Thanks
Robert
How many output rows do you have? Sorting can be expensive for large rowsets.
RBarryYoung
+1  A: 

Rewritten:

WITH base AS (
  SELECT i.ItemID,
         i.ItemRef,
         i.SiteID,
         i.ParentClass,
         i.Classification,
         i.Summary,
         i.Active,
         i.ItemTypeID,
         at.ActionTypeName 
    FROM ITEMS i
    JOIN ACTIONS a ON a.ItemID = i.ItemID
    JOIN ACTIONTYPES at ON at.ActionTypeID = a.ActionTypeID
   WHERE a.DateAndTime BETWEEN @startDate AND @endDate
     AND (@siteID = -1 OR i.SiteID = @siteID)
     AND (@parentClass = '%' OR i.ParentClass = @parentClass)
     AND (@class = '%' OR i.classification = @class)),
     items AS (
    SELECT b.ItemID,
           b.ItemRef,
           b.SiteID,
           b.ParentClass,
           b.Classification,
           b.Summary AS "Variant",
           b.Active
      FROM base b
     WHERE b.itemtypeid = 1
       AND b.actiontypename IN ('Full view', 'Print view', 'Email enquiry')
  GROUP BY i.ItemID, i.ItemRef, i.SiteID, i.ParentClass, i.Classification, i.Summary, i.Active),
     full_views AS (
    SELECT b.ItemID, 
           COUNT(*) AS num_full_Views
      FROM base b
      JOIN CUSTOMERSITES cs ON cs.siteid = b.siteid
      JOIN SITES s ON s.siteid = b.siteid
     WHERE b.itemtypeid = @itemTypeID
       AND b.ActionTypeName = 'Full view'
       AND (@customerID IS NULL OR cs.CustomerID = @customerID)
  GROUP BY b.itemid),
     print_views AS (
   SELECT b.ItemID, 
          COUNT(*) AS num_print_views
     FROM base b
     JOIN CUSTOMERSITES cs ON cs.siteid = b.siteid
     JOIN SITES s ON s.siteid = b.siteid
    WHERE b.itemtypeid = @itemTypeID
      AND b.ActionTypeName = 'Print view'
      AND (@customerID IS NULL OR cs.CustomerID = @customerID)
 GROUP BY b.itemid),
     email_queries AS (
 SELECT b.ItemID, 
        COUNT(*) AS num_email_enquiries
   FROM base b
   JOIN CUSTOMERSITES cs ON cs.siteid = b.siteid
  JOIN SITES s ON s.siteid = b.siteid
 WHERE b.itemtypeid = @itemTypeID
       AND b.ActionTypeName = 'Email enquiry'
       AND (@customerID IS NULL OR cs.CustomerID = @customerID)
  GROUP BY b.itemid)
   SELECT a.Active,
          a.ParentClass,
          a.Classification,
          a.Variant,
          ISNULL(fv.num_full_Views, 0) AS "Full Views",
          ISNULL(pv.num_print_views, 0) AS "Print Views",
          ISNULL(ee.num_email_enquiries, 0) AS "Email Enquiries",
          a.ItemRef,
          a.SiteID
     FROM items a
LEFT JOIN full_views fv ON fv.itemid = a.itemid
LEFT JOIN print_views pv ON pv.itemid = a.itemid
LEFT JOIN email_queries ee ON ee.itemid = a.itemid

To get better performance, I'd convert this to dynamic SQL in order to remove the parameter checks like these:

AND (@siteID = -1 OR i.SiteID = @siteID)

...because of the negative impact on sargability.

OMG Ponies
Working on that conversion now as it goes thanks.
Robert