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

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.