0

Stored Procedure

CREATE     procedure [dbo].[ImproveProcedure] (@port varchar(50), @portdate datetime)    
as    

Declare @intdate datetime    
select @intdate = max(rate_date) from Interestrate where rate_type = 'Zero'    
and rate_date <= @portdate    

Update transactiontable set NonDiscount = null, Discount = null, NonDiscountTcurr = null, DiscountTcurr = null,    
NonDiscountNew = null, DiscountNew = null    
where    
port = @port and portdate = @portdate    

Update tr set NonDiscount = (case sss_ind when 'P' then -exposure else exposure end) *    
      dbo.Foo(ccd, 'USD', @portdate,    
case when datediff(m, portdate, isnull(priceend1, isnull(priceend2, mature))) < 1 then 1     
 when datediff(m, portdate, isnull(priceend1, isnull(priceend2, mature))) > 48 then 48     
 else  datediff(m, portdate, isnull(priceend1, isnull(priceend2, mature)))     
end),    
NonDiscountTcurr = (case sss_ind when 'P' then -exposure else exposure end)    
from    
Phy p1    
where    
port = @port and portdate = @portdate    
and    
tr.trans = p1.trans    
and    
p1.Sub <> 'Option'    


Update tr set NonDiscount = (case when buysell in ('A', 'S') then -vol * markprice else vol * markprice end) *    
      dbo.Foo(ccd, 'USD', @portdate,     
case when datediff(m, portdate, isnull(priceend1, isnull(priceend2, mature))) < 1 then 1     
 when datediff(m, portdate, isnull(priceend1, isnull(priceend2, mature))) > 48 then 48     
 else  datediff(m, portdate, isnull(priceend1, isnull(priceend2, mature)))
end),    
NonDiscountTcurr = (case when buysell in ('A', 'S') then -vol * markprice else vol * markprice end)    
from    
Phy p1    
where    
port = @port and portfolio = @portfolio    
and    
tr.trans = p1.trans    
and    
p1.Sub = 'Option'





CREATE   function [dbo].[Foo]  
(@currency1 varchar(10),   
@currency2 varchar(10),  
@portdate datetime, @month int) returns float  

As  
BEGIN  
Declare  
@CurrentRate float,  
@Ratedate datetime  


select @Ratedate = max(rate_date) from fx where  
(  
currency1 = @currency1 and currency2 = @currency2 and forward_month = @month  
or  
currency2 = @currency1 and currency1 = @currency2 and forward_month = @month  
)  
and  
@portdate >= Rate_date  


IF exists ( select * from fx where currency1 = @currency1  
and currency2 = @currency2  and rate_date = @Ratedate and forward_month = @month)   


  SELECT @CurrentRate =  Rate from fx where currency1 = @currency1 and  
  currency2 = @currency2 and rate_date = @Ratedate and forward_month = @month  



ELSE   



  IF exists ( select * from fx where currency1 =  
  @currency2 and currency2 = @currency1 and rate_date = @Ratedate and forward_month = @month)  


  select @CurrentRate = 1/Rate  from fx  where currency1 = @currency2  
  and currency2 = @currency1 and rate_date = @Ratedate and forward_month = @month  


  ELSE  
   select @CurrentRate = 1   


return (@CurrRate )  
END  

I understand that scalar function calls can limit the performance of a query. Can someone see a way to remove the function calls, replacing them with a JOIN or CROSS APPLY or something instead, to see if this helps performance? I've tried, but haven't been able to find a way.

More info about current performance: A single run of the procedure, updating ~25,000 rows, takes 20-25 minutes to run. Looking at the execution plan, I see a lot of reads tied up in the function, and I'm thinking that with a JOIN solution (or some such), that number of reads would drop dramatically, and the performance may improve. Plus, as noted above, I've heard scalar function calls are (in general) "bad" in queries.

  • 2
    You indicate this is an attempt to improve performance. What is the performance like now? What lead you to believe changing the function will improve it? (Certainly not saying you're wrong, just useful to confirm that this makes sense) Does this procedure operate on one portfolio at a time (say, as part of a trigger when a record is inserted or updated), or is it operated in a cursor loop of some sort (in which case for best performance you'd want to move to a set-based solution all the way)? – RDFozz Oct 10 '17 at 16:06
  • I found this out by execution plan and it does alot of reads while a function call is made. It takes around 20-25 min to run to update 25000 rows in transactiontable. This one doesn't use CURSER or I want to avoid any loops. I want to achieve greater performance using the Joins or cross apply method. I am not able to figure out a way for it. – V_immo Oct 10 '17 at 16:33
  • 1
    I think you accidentally created two accounts. Please merge your accounts so you can edit this information into your question and respond to comments without creating these non-answers. – Erik Oct 10 '17 at 17:05
1

Here's an attempt to rewrite the function into the procedure:

CREATE PROCEDURE [dbo].[ImproveProcedure]
(
  @port varchar(50)
 ,@portdate datetime
)
AS   
BEGIN
    DECLARE @intdate datetime;
    SELECT @intdate = MAX(rate_date)
      FROM Interestrate
     WHERE rate_type = 'Zero'
       AND rate_date <= @portdate
    ;

    IF (OBJECT_ID('tempdb..#tmp_fx') IS NOT NULL) DROP TABLE #tmp_fx;
    CREATE TABLE #tmp_fx
         ( currency1 varchar(10)
          ,currency2 varchar(10)
          ,month int
          ,rate_date datetime
          ,rate float
          ,CONSTRAINT PK_tmp_fx PRIMARY KEY (currency2, month)
         );

    -- NOTE: currency1 not needed in PK, as it's always 'USD' - may want to change if add'l values are added.

    -- Get base currency pairs, months, and max dates
    INSERT INTO #tmp_fx
    SELECT currency1, currency2, forward_month, MAX(rate_date)
      FROM (SELECT currency1, currency2, forward_month, rate_date
              FROM fx
             WHERE currency1 = 'USD'
               AND rate_date <= @portdate
            UNION ALL
            SELECT currency2, currency1, forward_month, rate_date
              FROM fx
             WHERE currency2 = 'USD'
               AND rate_date <= @portdate
           ) sq
     GROUP BY currency1, currency2, forward_month
    ;

    -- Set rates based on previous query:
    --   Step 1: rates where currency1 = currency1 -- multiply
    UPDATE t
       SET rate = fx.Rate
      FROM #tmp_fx t
             INNER JOIN fx ON (    t.currency1 = fx.currency1
                               AND t.currency2 = fx.currency2
                               AND t.month = fx.forward_month
                               AND t.rate_date = fx.rate_date
                              )
    ;

    --   Step 2: rates where currency2 = currency1 -- divide
    UPDATE t
       SET rate = 1 / fx.Rate
      FROM #tmp_fx t
             INNER JOIN fx ON (    t.currency2 = fx.currency1
                               AND t.currency1 = fx.currency2
                               AND t.month = fx.forward_month
                               AND t.rate_date = fx.rate_date
                              )
     WHERE fx.rate IS NULL
    ;

    -- NOTE - unset rates left NULL - will force to 1 in final query


    UPDATE tr
       SET NonDiscount = null
          ,Discount = null
          ,NonDiscountTcurr = null
          ,DiscountTcurr = null
          ,NonDiscountNew = null
          ,DiscountNew = null    
     WHERE port = @port
       AND portdate = @portdate
    ;

    UPDATE tr
       SET NonDiscount =  CASE WHEN p1.Sub <> 'Option'
                            THEN CASE sss_ind
                                   WHEN 'P' then -exposure
                                   ELSE          exposure
                                 END
                            ELSE CASE WHEN buysell IN ('A', 'S')
                                   THEN -vol * markprice
                                   ELSE vol * markprice
                                 END
                          END
                        * ISNULL(fx.Rate, CAST(1 as float))
          ,NonDiscountTcurr = CASE WHEN p1.Sub <> 'Option'
                                THEN CASE sss_ind
                                       WHEN 'P' then -exposure
                                       ELSE          exposure
                                     END
                                ELSE CASE WHEN buysell IN ('A', 'S')
                                       THEN -vol * markprice
                                       ELSE vol * markprice
                                     END
                              END
      FROM tr
             INNER JOIN Phy p1 ON (tr.trans = p1.trans)
             LEFT  JOIN #tmp_fx fx ON (    'USD' = fx.currency1
                                       AND ccd = fx.currency2
                                       AND portdate = fx.rate_date
                                       AND  CASE 
                                              WHEN DATEDIFF(month, portdate, COALESCE(priceend1, priceend2, mature)) < 1
                                                THEN 1
                                              WHEN DATEDIFF(month, portdate, COALESCE(priceend1, priceend2, mature)) > 48
                                                THEN 48
                                              ELSE   DATEDIFF(month, portdate, COALESCE(priceend1, priceend2, mature))
                                            END
                                          = fx.month
                                      )
     WHERE port = @port
       AND portdate = @portdate
       AND p1.Sub IS NOT NULL
    ;
END;

Your function uses ccd, portdate, and the number of months between portdate and three possible dates (using the first that's not NULL) to calculate the rate that the exposure should be multiplied by.

  • If you can find a match where currency1 is 'USD', then you multiple by rate;
  • If the only match is where currency2 is 'USD', then you divide by rate;
  • if no match is found, the rate defaults to 1

So, let's try to build a table with your available rates in it.

We have basically have two factors that identify the rate_date values we'll use:

  • the month, an integer between 1 and 48;
  • the two currencies, one of which will be 'USD', the other of which will vary.

The maximum date for each of these will determine what rate is needed.

That gives us at most 48 * (number of currencies) rows to worry about, in practical terms. I'll guess that's 10-15 thousand rows, not an unreasonably large table.

For joining purposes, it will help to standardize the currency, so the first one is always USD - that way we don't have to worry about whether we're multiplying or dividing by the rate.

So, we build a temporary table with the unique values of currency1, currency2, and forward_month (always putting 'USD' in the temp table's currency1 column), and the maximum rate_date for each unique set of those three values.

Then, we add in the rates. In case it's relevant, we'll do it just like you did; If we have a case where 'USD' is in fx as currency1, we use that in preference over any rows where 'USD' is in currency2. Where 'USD' is in currrency1, we store rate directly; where it's in currency2, we need to divide by the rate, so we store 1 / rate.

Then, instead of using the function to find the rate, we join to the temporary table, based on:

  • the temp table's currency1 being 'USD'
  • the temp table's currency2 being the ccd from the other tables in the query
  • the rate_date matching the portdate column (which matches the passed-in @portdate)
  • the month matches the number of months between portdate, and the first of priceend1, priceend2, and mature that's not NULL. (Note: COALESCE takes a list of two or more values, and returns the first non-NULL value from the list; it's a little easier to read than ISNULL(x,ISNULL(y,z)), but works the same)

We make this a LEFT JOIN, so we include rows where we can't find a matching row in the temp table.

Then, we simply multiply the appropriate value (depending on the value of Sub) by the rate returned (or by 1, if we get back a NULL, either because the temp table had no match, or because the temp table had a matching row with a NULL rate).

NOTE: I combined your original two UPDATE statements into one, adding another CASE on the value of p1.Sub. To guarantee that this is working like your original two statement version, I also put in a check to see if p1.Sub is NULL; if that cannot be NULL, then you can remove that check.

Try this out and see if it works for you. It's not fully optimized (I think we could reduce the rows in the temp table by checking for currencies that are actually in our target transaction rows, for example) but it does remove the row-by-row nature of the function you were using.

NOTE: code is untested.

  • FYI - Updated code to match your revisions. Also, realized, I'd missed the statement dealing with p1.Sub = 'Option', so that's now been added. Left in my explanation - if there's a reason to remove, please respond here in the comments. – RDFozz Oct 19 '17 at 19:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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