Optimizing sales report stored procedure

Question

I need to optimize the following stored proc. Please let me know of any techniques or modifications that I can make to optimize this piece of code.

The procedure is for a report that needs to run and span a couple of months' worth of data.

ALTER PROCEDURE [dbo].[UP_Report_OverallSales_SP]
 @DateType INT = 1, --default date type to recognised date 
 @DateFrom DATETIME = null,
 @DateTo DATETIME = null ,
 @BrokerEmail VARCHAR(50) = null ,
 @EventName VARCHAR(50) = null,
 @SellerTypeList varchar(max) = '500,550,600', --default 'Large,Indy, Platinum'
 @AffiliateID INT = null,
 @AffiliateCategoryID INT = NULL,
 @StatusIDList varchar(max) = '2,3,5', --default 'Prcoessed,Completed, Dispatched'
 @FraudStatusIDList varchar(100) = '1', --default 'green'
 @LMSOrders BIT = 0
AS 
 BEGIN
 SET NOCOUNT ON
 Declare @ProcessedDateFrom DATETIME = NULL
 Declare @ProcessedDateTo DATETIME = NULL
 Declare @RecognisedDateFrom DATETIME = NULL
 Declare @RecognisedDateTo DATETIME = NULL
 IF @DateType = 1 -- 1 is recognised date 
 begin 
 set @RecognisedDateFrom = @DateFrom
 set @RecognisedDateTo = @DateTo
 end
 IF @DateType = 2 -- 2 is processed date 
 begin
 set @ProcessedDateFrom = @DateFrom
 set @ProcessedDateTo = @DateTo
 end
 if @AffiliateID is null 
 set @AffiliateID = -1
 if @AffiliateCategoryID is null 
 set @AffiliateCategoryID = -1
 Declare @BrokerId INT = null
 IF @EventName ='' 
 SELECT @EventName = NULL
 IF @BrokerEmail =''
 SELECT @BrokerEmail = NULL
 -- get broker id from the broker email passed as a parameter
 IF @BrokerEmail IS NOT NULL 
 Select @BrokerId = UserId from tbUser where email = @BrokerEmail
 --create seller type table 
 --this will have all comma separated seller type ids 
 Declare @SellerTypeTable Table (SellerTypeId int not null)
 If @SellerTypeList is not null 
 Insert into @SellerTypeTable (SellerTypeId) Select ID from dbo.FN_SplitToINT(@SellerTypeList, ',')
 --create status id table 
 --this will have all comma separated status id
 DECLARE @StatusIDTable TABLE (StatusID INT NOT NULL)
 If @StatusIDList is not null 
 INSERT INTO @StatusIDTable (StatusID) SELECT ID FROM dbo.FN_SplitToINT(@StatusIDList, ',')
 --create fraud status id table 
 --this will have all comma separated fraud status id 
 DECLARE @FraudStatusIDTable TABLE (FraudStatusID INT NOT NULL)
 If @FraudStatusIDList is not null 
 INSERT INTO @FraudStatusIDTable (FraudStatusID) SELECT ID FROM dbo.FN_SplitToINT(@FraudStatusIDList, ',')
 DECLARE @SaleWithDatesTable TABLE
 (
 SaleID INT NOT NULL,
 SaleDate DATETIME NULL,
 RecognisedDate DATETIME NULL,
 ProcessedDate DATETIME NULL,
 CompletedDate DATETIME NULL,
 CancellationDate DATETIME NULL,
 ShippedDate DATETIME NULL,
 DroppedDate DATETIME NULL,
 StatusID INT NULL,
 PRIMARY KEY (SaleID)
 )
 INSERT INTO @SaleWithDatesTable (SaleID)
 SELECT DISTINCT s.SaleID
 FROM
 tbSale s
 INNER JOIN tbTicketHistory th ON s.TicketHistoryID = th.TicketHistoryID
 WHERE 
 SaleID IN
 (
 select SaleID
 from tbSaleHistory 
 where ISNULL(FraudStatusID,1) in (select fraudstatusid from @FraudStatusIDTable)
 group by SaleID
 having MIN(UpdateTime) between @DateFrom AND @DateTo
 )
 AND (th.BrokerID = @BrokerID
 OR @BrokerID IS NULL)
 AND 
 (s.AffiliateID = @AffiliateID
 OR @AffiliateID = - 1)
 --print 'Table @SaleWithDatesTable'
 --select * from @SaleWithDatesTable
 -- 600 -- indy seller
 -- 550 -- platinum broker 
 -- 500 -- is broker 
 DECLARE @SaleIdsForIndySellersTable TABLE
 (
 SaleID INT NOT NULL,
 PRIMARY KEY (SaleID)
 )
 DECLARE @SaleIdsForPlatinumSellersTable TABLE
 (
 SaleID INT NOT NULL,
 PRIMARY KEY (SaleID)
 )
 -- insert all saleids that belong to indy seller in table @SaleIdsForIndySellersTable
 INSERT INTO @SaleIdsForIndySellersTable (SaleID)
 SELECT DISTINCT s.SaleID
 FROM tbSale s
 INNER JOIN [tbTicketHistory] th ON s.[TicketHistoryID] = th.[TicketHistoryID]
 INNER JOIN [tbUserRole] ur ON th.[BrokerID] = ur.[UserID] AND ur.[RoleID] = 600
 -- insert all saleids that belong to platinum seller in table @SaleIdsForPlatinumSellersTable
 INSERT INTO @SaleIdsForPlatinumSellersTable (SaleID)
 SELECT DISTINCT s.SaleID
 FROM tbSale s
 INNER JOIN [tbTicketHistory] th ON s.[TicketHistoryID] = th.[TicketHistoryID]
 INNER JOIN [tbUserRole] ur ON th.[BrokerID] = ur.[UserID] AND ur.[RoleID] = 550
 -- remove all saleids that do not belong to indy seller from @SaleWithDatesTable 
 -- if seller type selected do not have indy seller
 IF NOT EXISTS (SELECT 1 FROM @SellerTypeTable WHERE SellerTypeId = 600) -- INDY
 DELETE FROM @SaleWithDatesTable WHERE saleid IN (SELECT saleid FROM @SaleIdsForIndySellersTable) 
 -- remove all saleids that do not belong to platinum seller from @SaleWithDatesTable 
 -- if seller type selected do not have platinum seller 
 IF NOT EXISTS (SELECT 1 FROM @SellerTypeTable WHERE SellerTypeId = 550) -- PLATINUM
 DELETE FROM @SaleWithDatesTable WHERE saleid IN (SELECT saleid FROM @SaleIdsForPlatinumSellersTable) 
 -- remove all saleids that belong to neither platinum nor indy sellers
 -- if seller type selected do not have LARGE seller 
 IF NOT EXISTS (SELECT 1 FROM @SellerTypeTable WHERE SellerTypeId = 500) -- LARGE
 DELETE FROM @SaleWithDatesTable WHERE saleid NOT IN (SELECT saleid FROM @SaleIdsForIndySellersTable) 
 AND saleid not in (SELECT saleid FROM @SaleIdsForPlatinumSellersTable) 
 -- update dates in the table 
 UPDATE @SaleWithDatesTable SET RecognisedDate = SH2.UpdateTime
 from 
 (
 SELECT SH.SaleID, Min(SH.UpdateTime) as 'UpdateTime'
 from tbSaleHistory SH
 INNER JOIN @SaleWithDatesTable S ON SH.SaleID = S.SaleID
 where 
 ISNULL(SH.FraudStatusID,1) IN (select fraudstatusid from @FraudStatusIDTable) 
 -- SH.FraudStatusID IN (select fraudstatusid from @FraudStatusIDTable) 
 and SH.statusID in (select statusid from @StatusIDTable)
 GROUP BY SH.SaleID
 ) SH2
 INNER JOIN @SaleWithDatesTable S ON SH2.SaleID = S.SaleID
 UPDATE @SaleWithDatesTable SET ProcessedDate = ssd.UpdateTime FROM
 tbSaleStatusDate ssd
 INNER JOIN @SaleWithDatesTable s ON ssd.SaleID = s.SaleID
 WHERE ssd.StatusID = 2
 -----------------------------------------------------------------------------------
 UPDATE @SaleWithDatesTable SET CompletedDate = ssd.UpdateTime FROM
 tbSaleStatusDate ssd
 INNER JOIN @SaleWithDatesTable s ON ssd.SaleID = s.SaleID
 WHERE ssd.StatusID = 3
 -----------------------------------------------------------------------------------
 UPDATE @SaleWithDatesTable SET CancellationDate = ssd.UpdateTime FROM
 tbSaleStatusDate ssd
 INNER JOIN @SaleWithDatesTable s ON ssd.SaleID = s.SaleID
 WHERE ssd.StatusID = 4
 -----------------------------------------------------------------------------------
 UPDATE @SaleWithDatesTable SET ShippedDate = ssd.UpdateTime FROM
 tbSaleStatusDate ssd
 INNER JOIN @SaleWithDatesTable s ON ssd.SaleID = s.SaleID
 WHERE ssd.StatusID = 5
 -----------------------------------------------------------------------------------
 UPDATE @SaleWithDatesTable SET DroppedDate = ssd.UpdateTime FROM
 tbSaleStatusDate ssd
 INNER JOIN @SaleWithDatesTable s ON ssd.SaleID = s.SaleID
 WHERE ssd.StatusID = 9
 -----------------------------------------------------------------------------------
 UPDATE @SaleWithDatesTable SET SaleDate = ssd.UpdateTime FROM
 tbSaleStatusDate ssd
 INNER JOIN @SaleWithDatesTable s ON ssd.SaleID = s.SaleID
 WHERE ssd.StatusID = 1
 --print @RecognisedDateFrom 
 --print @RecognisedDateTo
 --select * from @SaleWithDatesTable
 IF @RecognisedDateFrom IS NOT NULL 
 DELETE FROM @SaleWithDatesTable WHERE RecognisedDate < @RecognisedDateFrom or RecognisedDate IS NULL
 IF @RecognisedDateTo IS NOT NULL 
 DELETE FROM @SaleWithDatesTable WHERE RecognisedDate > @RecognisedDateTo or RecognisedDate IS NULL
 IF @ProcessedDateFrom IS NOT NULL 
 DELETE FROM @SaleWithDatesTable WHERE ProcessedDate < @ProcessedDateFrom or ProcessedDate IS NULL
 IF @ProcessedDateTo IS NOT NULL 
 DELETE FROM @SaleWithDatesTable WHERE ProcessedDate > @ProcessedDateTo or ProcessedDate IS NULL
 select TSH.* 
 into #tmptbSaleHistory
 from tbSaleHistory TSH
 left Join @SaleWithDatesTable SD ON (SD.saleid = TSH.saleid)
 WHERE TSH.StatusID IN (Select StatusID from @StatusIDTable)
 and TSH.saleHistoryID in
 (select min(saleHistoryID) from tbSaleHistory where StatusID IN (Select StatusID from @StatusIDTable)and SaleID in (select SaleID from @SaleWithDatesTable) group by SaleID)
 SELECT DISTINCT
 TS.ApplicationID,
 TAL.NAME AS 'ApplicationName',
 TS.SaleID,
 SD.SaleDate AS 'Saledate',
 SD.RecognisedDate AS 'RecognisedDate',
 SD.ProcessedDate AS 'ProcessedDate',
 SD.CompletedDate AS 'CompletedDate',
 SD.CancellationDate AS 'CancellationDate',
 SD.ShippedDate AS 'ShippedDate',
 SD.DroppedDate AS 'DroppedDate',
 TSS.[ShowDate],
 TTH.BrokerID,
 ISNULL(TA2.Company,'') AS 'Broker',
 TEC.[Name] AS 'EventName',
 TEC.EventID,
 TSH.Quantity,
 CAST(ROUND((TSH.SellAmount / ((100 + TSH.MarkupRate) / 100)) / TSH.ExchangeRateToGBP, 2) AS decimal(10,2)) AS 'TicketAmountWoMarkup', --for Tiered fee no change
 CAST(ROUND((TSH.Quantity * TSH.SellAmount / ((100 + TSH.MarkupRate) / 100)) / TSH.ExchangeRateToGBP, 2) as decimal(10,2)) AS 'TotalTicketAmountWoMarkup', --for Tiered fee no change
 TSH.MarkupRate,
 TSH.InitialMarkupRate,
 CASE WHEN TSH.FeeTypeID = 1
 THEN
 CAST(ROUND(((TSH.Quantity * (TSH.MarkupRate / (100 + TSH.MarkupRate) * TSH.SellAmount)) + TSH.TieredProcessingFee) / TSH.ExchangeRateToGBP, 2) as decimal(10,2))
 ELSE
 CAST(ROUND((TSH.Quantity * (TSH.MarkupRate / (100 + TSH.MarkupRate) * TSH.SellAmount)) / TSH.ExchangeRateToGBP, 2) as decimal(10,2))
 END AS 'MarkupAmount',
 CAST(ROUND(TSH.ShippingAmount / TSH.ExchangeRateToGBP, 2) as decimal(10,2)) AS 'ShippingAmount',
 TSH.TaxRate,
 CAST(ROUND(TSH.TaxAmount / TSH.ExchangeRateToGBP, 2) as decimal(10,2)) AS 'TaxAmount',
 CAST(ROUND(TSH.DiscountAmount / TSH.ExchangeRateToGBP, 2) as decimal(10,2)) AS 'DiscountAmount',
 CAST(ROUND(TSH.ServiceChargeAmount / TSH.ExchangeRateToGBP, 2) as decimal(10,2)) AS 'ServiceChargeAmount',
 CASE WHEN TSH.FeeTypeID = 1
 THEN
 CAST(ROUND((((TSH.Quantity * TSH.SellAmount) + TSH.TieredProcessingFee) + TSH.ShippingAmount) / TSH.ExchangeRateToGBP, 2) as decimal(10,2))
 ELSE
 CAST(ROUND((TSH.Quantity * TSH.SellAmount + TSH.ShippingAmount) / TSH.ExchangeRateToGBP, 2) as decimal(10,2))
 END AS 'APamount',
 CAST(ROUND((TSH.Quantity * TSH.SellAmount / ((100 + TSH.MarkupRate) / 100) + TSH.ShippingAmount) / TSH.ExchangeRateToGBP, 2) as decimal(10,2)) AS 'ARamount', --for Tiered fee no change
 CAST(ROUND(TSH.TotalAmount / TSH.ExchangeRateToGBP, 2) as decimal(10,2)) AS 'TotalAmount',
 TS.StatusID,
 TSSL.Description AS 'Status',
 tvc.venuename 'Venue',
 TCCV.CityName VenueCity,
 ISNULL(TS.FraudStatusID,1) as 'FraudStatusID',
 TTH.FaceAmount,
 TCML.currencyName as 'CurrencyName',
 CASE 
 WHEN EXISTS (SELECT 1 FROM tbUserRole AS UR
 WHERE (UR.RoleID = 600) AND (UR.UserID = TTH.BrokerID))
 THEN 'I'
 WHEN EXISTS (SELECT 1 FROM tbUserRole AS UR
 WHERE (UR.RoleID = 550) AND (UR.UserID = TTH.BrokerID))
 THEN 'P'
 WHEN EXISTS (SELECT 1 FROM tbUserRole AS UR
 WHERE (UR.RoleID = 500) AND (UR.UserID = TTH.BrokerID))
 THEN 'L'
 ELSE 'NA'
 END AS 'SellerType',
 ISNULL(TS.Lock,0) as 'IsLocked',
 TSH.CommissionRate,
 TSH.CommissionAmount,
 TS.ShippingMethodID,
 TS.ShippingText,
 TS.TrackingNumber,
 TS.IsPaid,
 TU.PayPalID,
 PML.Description as 'PaymentMethod'
 INTO
 #T1
 FROM
 @SaleWithDatesTable SD 
 INNER JOIN [tbSale] TS (NOLOCK) ON TS.SaleID = SD.SaleID
 INNER JOIN #tmptbSaleHistory TSH ON TSH.saleID = SD.SaleID
 INNER JOIN tbTicketHistory TTH ON TS.TicketHistoryID = TTH.TicketHistoryID
 INNER JOIN tbshow TSS ON TSS.[ShowID] = TTH.[ShowID]
 INNER JOIN [tbSaleStatusLookup] TSSL ON TSSL.[SaleStatusID] = TS.[StatusID]
 INNER JOIN tbVenue TV ON TSS.VenueID = TV.VenueID
 INNER JOIN [tbApplicationLookup] TAL ON TAL.ApplicationID = TS.ApplicationID
 INNER JOIN [tbEvent] TEC ON TEC.[EventID] = TSS.[EventID]
 LEFT OUTER JOIN [tbUserAddress] TUA2 ON TUA2.[UserID] = tth.[BrokerID]
 AND TUA2.[AddressTypeID] = 4 AND TUA2.isdefault=1
 LEFT OUTER JOIN [tbAddress] TA2 ON TUA2.[AddressID] = TA2.[AddressID] 
 LEFT OUTER JOIN dbo.tbAddress A ON TV.VenueAddressID = A.AddressID
 LEFT OUTER JOIN dbo.tbCityCulture TCCV ON A.CityID = TCCV.CityID
 AND TCCV.CultureID = 1
 LEFT OUTER JOIN [tbVenueCulture] TVC ON tvc.venueid = tss.venueid
 AND tvc.cultureid = 1
 LEFT OUTER JOIN [tbCurrencyMasterLookup] TCML ON TCML.currencyid = TS.CurrencyID
 LEFT OUTER JOIN [tbUser] TU ON TU.[UserID] = TS.[BuyerID]
 LEFT OUTER JOIN tbSalePaymentReceived SPR ON SPR.SaleID = TS.SaleID
 LEFT OUTER JOIN tbPaymentReceived PR ON SPR.PaymentID = PR.PaymentID
 LEFT OUTER JOIN tbPaymentMethodLookup PML ON PML.PaymentMethodID = PR.PaymentMethodID
 LEFT OUTER JOIN [tbAffiliate] TAA ON TAA.affiliateID = TS.affiliateID 
 LEFT OUTER JOIN tbaffiliatecategory TAC ON TAA.affiliateCategoryID = TAC.CategoryID 
 WHERE 
 (@EventName IS NULL
 OR TEC.[Name] LIKE @EventName)
 -- AND (@EventTypeID IS NULL OR TEC.EventTypeID = @EventTypeID)
 AND (TAA.AffiliateCategoryID = @AffiliateCategoryID or @AffiliateCategoryID = -1)
 AND (TS.FraudStatusID IN (select fraudstatusid from @FraudStatusIDTable))
 AND (TS.StatusID IN (select StatusID from @StatusIDTable)) 
 AND (TTH.IsLMS = @LMSOrders)
 SELECT
 *
 FROM #t1 AS t
 ORDER BY
 t.RecognisedDate
 DROP TABLE [#T1]
 DROP TABLE #tmptbSaleHistory
 END

• Mattias Åslund: Optimize for speed, readability or something else?

Answer 1

Be consistent with your naming and Capitalization of Keywords.

Some places you write null and others you write NULL, some places Declare other places DECLARE. Personally I write them all in uppercase, it's a pain, but is so much easier to read in my opinion, especially when it is in plain text and not in an IDE.

You do some weird things with variables that you don't have to do, like setting them if they weren't set, you use defaults on other input parameters but not on some of them.

if @AffiliateID is null 
 set @AffiliateID = -1
if @AffiliateCategoryID is null 
set @AffiliateCategoryID = -1

Just declare the variables with a default of -1 instead of NULL

Kind of the same thing here

IF @EventName ='' 
 SELECT @EventName = NULL
IF @BrokerEmail =''
 SELECT @BrokerEmail = NULL
-- get broker id from the broker email passed as a parameter
IF @BrokerEmail IS NOT NULL 
 Select @BrokerId = UserId from tbUser where email = @BrokerEmail

@EventName is already NULL there is no reason to set it to NULL.

@BrokerEmail is always NULL here, it starts as NULL so it skips the first if statement and then skips the next if statement because it is NULL and is not used (skimmed the rest of the code and didn't see it), what are we doing with this?

---

@SellerTypeList is never going to be NULL because you gave the parameter defaults so drop the if statement on this and just create the table

DECLARE @SellerTypeTable Table (SellerTypeId INT NOT NULL)
IF @SellerTypeList IS NOT NULL 
 INSERT INTO @SellerTypeTable (SellerTypeId) SELECT ID FROM dbo.FN_SplitToINT(@SellerTypeList, ',')

---

Could you create a single table that can be used for other things if not used for SellerTypes?

Like with these 2 tables

--create status id table 
--this will have all comma separated status id
DECLARE @StatusIDTable TABLE (StatusID INT NOT NULL)
If @StatusIDList IS NOT NULL 
 INSERT INTO @StatusIDTable (StatusID) SELECT ID FROM dbo.FN_SplitToINT(@StatusIDList, ',')
--create fraud status id table 
--this will have all comma separated fraud status id 
DECLARE @FraudStatusIDTable TABLE (FraudStatusID INT NOT NULL)
If @FraudStatusIDList is not NULL 
 INSERT INTO @FraudStatusIDTable (FraudStatusID) SELECT ID FROM dbo.FN_SplitToINT(@FraudStatusIDList, ',')

Can you use a single Table with an extra column that specifies if they are a FraudStatus or a Status? it would be just as easy to go through the table when you add a where clause on that column.

---

Table names using Hungarian Notation, SIGH, probably nothing you can do about that.

Aliasing tables with one or two letters is horrible, use a full name so you know what table you are dealing with.

---

Do you really want to update every record like this?

UPDATE @SaleWithDatesTable SET RecognisedDate = SH2.UpdateTime
from 
(
 SELECT SH.SaleID, Min(SH.UpdateTime) as 'UpdateTime'
 from tbSaleHistory SH
 INNER JOIN @SaleWithDatesTable S ON SH.SaleID = S.SaleID
 where 
 ISNULL(SH.FraudStatusID,1) IN (SELECT fraudstatusid from @FraudStatusIDTable)
 -- SH.FraudStatusID IN (SELECT fraudstatusid from @FraudStatusIDTable) 
 and SH.statusID in (SELECT statusid from @StatusIDTable)
 GROUP BY SH.SaleID
) SH2
 INNER JOIN @SaleWithDatesTable S ON SH2.SaleID = S.SaleID

of all the places that you write comments, this should be one where you actually need a comment in the code to let people know why you are updating every row in the table.

---

 TS.StatusID,
 TSSL.Description AS 'Status',
 tvc.venuename 'Venue',
 TCCV.CityName VenueCity,
 ISNULL(TS.FraudStatusID,1) as 'FraudStatusID',
 TTH.FaceAmount,
 TCML.currencyName as 'CurrencyName',

What table is TTH? ... wait I finally found it in this massive query it's tbTicketHistory how about TicketHistory

TSSL? how about SaleStatLookUp

Believe me, writing it the first time is a pain in the neck when you have to write them all out, but when you come back in 5 months and can tell exactly what is going on in the select with out having to search to find what the heck TCCV is supposed to be, you will be glad that you wrote out the table names.

---

Put some space between Select statements. That great big query is followed immediately by another small query that I almost didn't see because there wasn't any new lines between them.

And that extra query is almost useless as well, all it does is order the Temp Table that you created with the big query, Lose the Temp Table #t1 and add the ORDER BY after the WHERE clause.

---

That should be a good start, I am sure there is more that can be fixed here though.

I suggest making these changes, clean it up a bit, make sure it runs, and then post another question with the revised code.