Any ideas how to optimize this batch from a third party?

Hans 2013-12-30 00:41:39

Aaron Bertrand 2013-12-30 03:05:15
With only an estimated plan, I can only have so much insight into exactly how these things will help, but I do have several ideas. I tried to comment all of my changes inline, and I'm sorry if there are syntax errors (I couldn't really validate without schema). I am also pretty sure you can do this without a cursor, but I only have so much time I can spend on a Sunday night reverse engineering your code. 🙂

CREATE PROCEDURE dbo.spResRapTeoretiskKonsern
  @Aarstall  INT, 
  @Maned     INT, 
  @Konsern   VARCHAR(10), 
  @Avdeling  VARCHAR(50), 
  @Region    VARCHAR(10), 
  @Distrikt  VARCHAR(10)
AS
BEGIN
  -- always use SET NOCOUNT ON
  SET NOCOUNT ON;
 
  -- why not just say DECLARE once?
  DECLARE @DataAreaID              VARCHAR(50),
          @SumInntekter            FLOAT,
          @Varekjop                FLOAT,
          @VarekjopTeoretisk       FLOAT,
          @Driftskostnader         FLOAT,
          @Driftsresultat          FLOAT,
          @NettoFinans             FLOAT,
          @ResultatSkatt           FLOAT,
          @OrdinaertResultat       FLOAT,
          @AarsResultat            FLOAT,
          @VirkeligVarebeholdning  FLOAT,
          @TeoretiskVareBeholdning FLOAT,
          @VarelagerEndring        FLOAT,
          @FirmaNavn               VARCHAR(50);
 
    CREATE TABLE #TMP_rsProfitAndLoss 
    (
      Sorting         VARCHAR(10)
    , AccountName     VARCHAR(100)
    , AccountNum      VARCHAR(50)
    , DataAreaId      VARCHAR(50)
    , RsReportGroup   VARCHAR(60)
    , Transdate       DATETIME
    , Prosentsats     FLOAT
    , Sum1            FLOAT
    , ReportID        VARCHAR(50)
    , Salgsinntekter1 FLOAT
    , Prosent1        FLOAT
    , Konsern         VARCHAR(10)
    , Firmanavn       VARCHAR(50)
    ); -- always use semi-colons; see
    -- http://sqlblog.com/blogs/aaron_bertrand/archive/2009/09/03/ladies-and-gentlemen-start-your-semi-colons.aspx
 
    -- get rid of the IF conditionals. 
    -- you can just change the DIMENSION = clause slightly to support wildcard
    -- (or you can run the SELECT using dynamic SQL to avoid confusing the optimizer)
 
    INSERT INTO #TMP_rsProfitAndLoss
    SELECT SORTING, LEDGERTABLEACCOUNTNAME, LEDGERTABLEACCOUNTNUM, [DATAAREAID#5], 
      REPORTGROUPNAME, TRANSDATE,
      (
        -- this TOP 1 without ORDER BY seems fishy to me
        -- I am sure you could make this a proper join
        SELECT TOP 1 (Case when (percent_ is null) then 0 else percent_ end) 
        FROM dbo.IKLRSTHEORETICALGROSSPROFIT -- always use schema prefixes, see
          -- http://sqlblog.com/blogs/aaron_bertrand/archive/2009/10/11/bad-habits-to-kick-avoiding-the-schema-prefix.aspx
        WHERE companyid = p.[Dataareaid#5] AND dimension = ''
        AND p.transdate >= FromDate AND p.transdate <= ToDate)
      ) AS Prosentsats,
      (Case when (year(transdate)=@Aarstall AND month(transdate)<=@Maned) then amountmst*-1 else 0 end) AS Sum1,
      ReportID, '', 0, c.IMPORTVATNUMBRANCHID, c.ADBLEDCOMPANYNAMEMARKET
      FROM dbo.RSPROFITANDLOSS AS p
        JOIN dbo.COMPANYINFO  AS c 
        ON p.[DATAAREAID#5] = c.DATAAREAID
        -- use table aliases and specify which table these columns come from!
      WHERE ReportID = 'KRES' AND (year(transdate)=@Aarstall AND month(transdate)<=@Maned) 
      AND (c.IMPORTVATNUMBRANCHID = @Konsern) 
      AND DIMENSION LIKE @Avdeling 
      AND iklCompanyRegionId LIKE @Region 
      AND iklCompanyDistrictId LIKE @Distrikt
      ORDER BY LEDGERTABLEACCOUNTNUM;
 
    -- include sum1 in the index to avoid RID lookups
    -- also note that DataAreaId should be the leading column
    CREATE INDEX IX_1 on #TMP_rsProfitAndLoss (DataAreaId, Sorting) INCLUDE(sum1);
 
    -- alternatively, make this a clustered index
    -- CREATE CLUSTERED INDEX IX_1 ON #TMP_rsProfitAndLass(DataAreaId, Sorting);
 
    -- always use LOCAL FAST_FORWARD on firehose cursors; see
    -- http://www.sqlperformance.com/2012/09/t-sql-queries/cursor-options
 
    DECLARE DataAreAId_cursor CURSOR LOCAL FAST_FORWARD
    FOR
        SELECT DISTINCT DataAreaId
        FROM #TMP_rsProfitAndLoss;
 
    OPEN DataAreAId_cursor;
 
    FETCH NEXT FROM DataAreAId_cursor INTO @DataAreaID
    WHILE (@@FETCH_STATUS <> -1)
    BEGIN
        IF (@@FETCH_STATUS <> -2)
        BEGIN    
          -- combine these to only perform one scan
            SELECT 
              @SumInntekter      = SUM(CASE WHEN Sorting = '13'  THEN Sum1 END),
              @Driftskostnader   = SUM(CASE WHEN Sorting = '29'  THEN Sum1 END),
              @NettoFinans       = SUM(CASE WHEN Sorting = '45'  THEN Sum1 END), 
              @Varekjop          = SUM(CASE WHEN Sorting = '21'  THEN Sum1 END),
              @VarekjopTeoretisk = SUM(CASE WHEN Sorting = '11'  THEN ((Sum1 - (Sum1 * Prosentsats)/100) * -1) END),
              @OrdinaertResultat = SUM(CASE WHEN Sorting = '47'  THEN Sum1 END),
              @AarsResultat      = SUM(CASE WHEN Sorting = '200' THEN Sum1 END),
              @FirmaNavn         = MAX(FirmaNavn) -- making a guess here that MAX will be acceptable
            FROM #TMP_rsProfitAndLoss WHERE DataAreaID = @DataAreaID;
 
            IF (@VarekjopTeoretisk <> 0)
            BEGIN
                SET @Driftskostnader = (@Driftskostnader + @VarekjopTeoretisk - @Varekjop);
                SET @Driftsresultat = (@SumInntekter + @Driftskostnader);
                SET @ResultatSkatt = (@Driftsresultat + @NettoFinans);
 
                -- we can do this without verbose IF logic and without checking
                -- for an empty string - not sure how a SUM could yield '':
                SELECT @OrdinaertResultat = COALESCE(@OrdinaertResultat, 0),
                       @AarsResultat = COALESCE(@AarsResultat, 0);
 
                SELECT @OrdinaertResultat += @ResultatSkatt,
                       @AarsResultat += @OrdinaertResultat;
 
                -- we can also combine these into a single statement:
 
                DELETE #TMP_rsProfitAndLoss WHERE DataAreaID = @DataAreaID
                              AND SORTING IN ('21','29','30','46','48','49');
 
        SELECT @VirkeligVarebeholdning = SUM(amountmst) 
          FROM dbo.ledgertrans WHERE (accountnum = '1460' 
          AND year(transdate)=@Aarstall AND month(transdate)<=@Maned AND DATAAREAID = @DataAreaID);
 
        --For å sikre at det kommer ut tall på de uten teoretiske beholdninger også.
        if @VirkeligVarebeholdning = '' SET @VirkeligVarebeholdning = 0;
        if @Varekjop = '' SET @Varekjop = 0;
        if @VarekjopTeoretisk = '' SET @VarekjopTeoretisk = 0;
 
        SET @TeoretiskVareBeholdning = @VirkeligVarebeholdning - @Varekjop + @VarekjopTeoretisk;
        SET @VarelagerEndring = @TeoretiskVareBeholdning - @VirkeligVarebeholdning;
 
          INSERT INTO #TMP_rsProfitAndLoss
          (SORTING, AccountName, AccountNum, RsReportGroup, Firmanavn, Sum1, DataAreaID)
          VALUES
          ('21', 'Varekostnad', '0000', 'Varekostnad', @FirmaNavn, 0, @DataAreaID),
          ('29', 'SUM DRIFTSKOSTNADER', '0000', 'SUM DRIFTSKOSTNADER', @FirmaNavn, @Driftskostnader, @DataAreaID),
          ('30', 'DRIFTSRESULTAT', '0000', 'DRIFTSRESULTAT', @FirmaNavn, @Driftsresultat, @DataAreaID),
          ('46', 'RESULTAT FØR SKATT', '0000', 'RESULTAT FØR SKATT', @FirmaNavn, @ResultatSkatt, @DataAreaID),
          ('48', 'ORINÆRT RESULTAT', '0000', 'ORINÆRT RESULTAT', @FirmaNavn, @OrdinaertResultat, @DataAreaID),
          ('49', 'ÅRSRESULTAT', '0000', 'ÅRSRESULTAT', @FirmaNavn, @AarsResultat, @DataAreaID),
          ('22', 'Varekostnad Teoretisk', '0000', 'Varekostnad Teoretisk', @FirmaNavn, @VarekjopTeoretisk, @DataAreaID),
          ('97', 'Teoretisk varebeholdning', '0000', 'Teoretisk varebeholdning', @FirmaNavn, @TeoretiskVareBeholdning, @DataAreaID),
          ('98', 'Varebeholdning ved siste telling', '0000', 'Varebeholdning ved siste telling', @FirmaNavn, @VirkeligVarebeholdning, @DataAreaID),
          ('99', 'Varelager endring', '0000', 'Varelager endring', @FirmaNavn, @VarelagerEndring, @DataAreaID);
        END
        FETCH NEXT FROM DataAreAId_cursor INTO @DataAreaID;
    END
    DELETE #TMP_rsProfitAndLoss WHERE SORTING = '200'; --Brukes kun for å bygge opp rapporten
 
    CLOSE DataAreAId_cursor;
    DEALLOCATE DataAreAId_cursor;
 
    SELECT * FROM #TMP_rsProfitAndLoss 
      ORDER BY DataAreaId, Sorting;
 
    DROP TABLE #TMP_rsProfitAndLoss;
END
Hans 2014-03-21 08:27:18
Thank you Aaron for your very good answer, a have read and used, it and should have thanked you a long time ago.