August 13, 2010 at 9:28 am
i am looking for a way to speed up this piece of code.
It is a simple while loop - which is repeated for every customer in the @CustomerLoopTbl
It obtains the AmountFinal from a function (dbo.Fn_GetAmount) This function must accept a recordDate parameter.
Now that the AmountFinal has been obtained, apply this value to the #TempResults table
Is there any way to code this setbased and avoid using a loop?
WHILE (SELECT COUNT(CustomerCode) FROM @CustomerLoopTbl) > 0
BEGIN
SELECT TOP(1) @CustomerCode= CustomerCode,
@RecordDate = LastDate
FROM @CustomerLoopTbl
SELECT @AmountLC = AmountFinal FROM dbo.Fn_GetAmount(@RecordDate,1) WHERE CustomerCode = @CustomerCode
UPDATE #TempResults
SET Amount_Lastest = @AmountLC WHERE CustomerCode = @CustomerCode
DELETE TOP(1) FROM @CustomerLoopTbl --> to proceed to next record.
END
August 13, 2010 at 9:45 am
Next time, please supply DDL of your objects and some test data (as per link found in the bottom of my signature).
You will need to change you table-valued function to accept CustomerCode as parameter, then you can do this:
UPDATE TR
SET Amount_Lastest = FGA.AmountFinal
FROM #TempResults AS TR
JOIN @CustomerLoopTbl AS C
ON C.CustomerCode = TR.CustomerCode
CROSS APPLY dbo.Fn_GetAmount(C.RecordDate,C.CustomerCode, 1) AS FGA
It is no going to be very fast anyway. If you post your UDF defenition, it may be possible to write set-based operation without using it.
August 13, 2010 at 2:02 pm
His function already returns the CustomerCode, so he can join on that instead of rewriting the function.
UPDATE TR
SET Amount_Lastest = FGA.AmountFinal
FROM #TempResults AS TR
INNER JOIN dbo.Fn_GetAmount(TR.RecordDate, 1) AS FGA
ON FGA.CustomerCode = TR.CustomerCode
This, of course, assumes that RecordDate is one of the fields in #TempResults, which we would know if the OP had posted DDL and sample data.
Drew
J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA
August 16, 2010 at 7:19 am
Thanks for the replies , Drew and Eugene.
RecordDate is a field on #TempResults - see definition below
ive tried using Drew's idea method but have found that you cannot reference the TR.RecordDate as a parameter in the function call
error message is:
Msg 4104, Level 16, State 1, Line 249
The multi-part identifier "TR.RecordDate" could not be bound.
(This is initially the reason why i used a While loop so i could assign the function parameter to a date-variable - as in my code.)
Ps : Using a CROSS APPLY is not an option as the exec time is much longer than using a While loop
Drop Table #TempResults
CREATE TABLE #TempResults
(
CustomerCode Varchar(20),
RecordDate SmallDateTime,
Amount_Lastest decimal(18,2)
)
--basic test data:
Insert Into #TempResults
SELECT '10','12 Jul 2010', 55012.22
UNION ALL
SELECT '31','13 Jul 2010', 40212.87
UNION ALL
SELECT '34','14 Jul 2010', 100000.00
UNION ALL
SELECT '136','15 Jul 2010', 500000.00
@CustomerLoopTbl is a copy of #TempResults (without AmountLastest field)
August 16, 2010 at 7:29 am
What about DDL for your tables and UDF? Without these, no much futher to advise. Most likely it is possible to replace your loop with better performing set-base code. But, without much details from you, it is hard to help.
August 16, 2010 at 7:46 am
The function is actually outside the scope of my question and , in fact cannot be changed and is written as efficiently as possilble.
Thanks
August 16, 2010 at 8:10 am
Without seeing all picture how would anyone know that
"CustomerLoopTbl is a copy of #TempResults (without AmountLastest field)" and it exists only in order for controlling your loop? Ok, name does suggest it kind of think, but doesn't guarantee this, as you could preselect records from different sources to drive your update.
Do you know what makes query slow, CROSS APPLY or "now-not-required" JOIN to the table variable?
I guess you can use this one (you don't need copy of #TempResults anymore) :
UPDATE TR
SET Amount_Lastest = FGA.AmountFinal
FROM #TempResults AS TR
CROSS APPLY dbo.Fn_GetAmount(TR.RecordDate,TR.CustomerCode, 1)
AS FGA
ron5 (8/16/2010)
The function is actually outside the scope of my question and , in fact cannot be changed and is written as efficiently as possilble....
Looking into LOOP implementation for UPDATE, one need to be very optimistic in assuming the above.: -D
Please follow the forum etiquette devised by Jeff Moden to post your questions in a future (link at the bottom of my signature), it guarantees that you will have more relevant help in a timly manner and reduces time for guessing wasted by your helpers.
August 16, 2010 at 9:58 am
I agree with Eugene. If we can get a sense of the whole picture, we would have a better change of providing a more efficient set-based solution. What if the customer is in the table twice? What would the expected results be? Is that function actually selecting from the same table? What is it doing? How does the temp table get populated?
For better, quicker answers, click on the following...
http://www.sqlservercentral.com/articles/Best+Practices/61537/
For better answers on performance questions, click on the following...
http://www.sqlservercentral.com/articles/SQLServerCentral/66909/
August 24, 2010 at 10:03 am
hi.
Ive simplified all the code so its easier to illustrate.
For each customer-record in #TempResults, the function Fn_GetAmount is called. The method used to do each function call is in a While-loop. I am wondering if there is an efficient set based method to perform this rather than looping. I find that with similar challenges , i usually resort to a similar while-loop structure which sometimes can be ineficient (I have tried a CROSS JOIN to the function, but this is far too expensive.)
below is all the code
create FUNCTION [dbo].[Fn_GetAmount] --
(
@DateAsAt datetime, -- A given date for which data must be queried on
@SetId int -- Id of price
)
RETURNS @Results TABLE
(
[AmountFinal] decimal(18,2) , -->amount for new taxyear
[CustomerCode] varchar(20)
) AS
BEGIN
if @DateAsAt < '31 mar 2010' and @DateAsAt > '28 feb 2009'
Begin
insert into @Results
Select 10000000 , '136'
insert into @Results
Select 50000 , '34'
insert into @Results
Select 40500 , '10'
End
Else
Begin
insert into @Results
Select 20000000 , '136'
insert into @Results
Select 100000 , '34'
insert into @Results
Select 81000 , '10'
End
RETURN
END
Drop Table #TempResults
CREATE TABLE #TempResults
(
CustomerCode Varchar(20),
RecordDate SmallDateTime,
Amount_Latest decimal(18,2)
)
--basic data:
Insert Into #TempResults
SELECT '10','12 Jul 2010', 55012.22
UNION ALL
SELECT '31','13 Jul 2010', 40212.87
UNION ALL
SELECT '34','14 Jul 2010', 100000.00
UNION ALL
SELECT '136','15 Jul 2010', 500000.00
DECLARE @CustomerLoopTbl TABLE(
CustomerCode varchar(50),
LastDate smalldatetime
)
/*create a table to loop for each Customer*/
INSERT INTO @CustomerLoopTbl
SELECT
CustomerCode,
RecordDate
FROM
#TempResults
DECLARE @CustomerCode varchar(50)
DECLARE @RecordDate smalldatetime
declare @AmountLC decimal(18,2)
WHILE (SELECT COUNT(CustomerCode) FROM @CustomerLoopTbl) > 0 -->until nomore records in looping table.
BEGIN
SELECT TOP(1) @CustomerCode= CustomerCode,
@RecordDate = LastDate
FROM @CustomerLoopTbl
SELECT @AmountLC = AmountFinal FROM dbo.Fn_GetAmount(@RecordDate,1) WHERE CustomerCode = @CustomerCode
UPDATE #TempResults
SET Amount_Latest = @AmountLC WHERE CustomerCode = @CustomerCode
DELETE TOP(1) FROM @CustomerLoopTbl --> delete this to proceed to next record.
END
select * from #TempResults
August 24, 2010 at 10:33 am
Are the results hardcoded like that for specific CustomerCodes? I put my Customer data in a table with start and end dates. The code you specified is updating Customer 31, but nothing is specified in the function for that customer, so I'm not sure why. I think it has to do with how you are deleting them from the temp table. It'd would be easier to put an identity column in the table and then get the rowcount of the table. Then you could just start with @rownum = 1 and increment until the end (While @rownum <= Rowcount). Anyway, is this code close to what you are looking for?
Drop Table #TempResults
CREATE TABLE #TempResults
(
CustomerCode Varchar(20),
RecordDate SmallDateTime,
Amount_Latest decimal(18,2)
)
--basic data:
Insert Into #TempResults
SELECT '10','12 Jul 2010', 55012.22
UNION ALL
SELECT '31','13 Jul 2010', 40212.87
UNION ALL
SELECT '34','14 Jul 2010', 100000.00
UNION ALL
SELECT '136','15 Jul 2010', 500000.00
Drop Table #CustomerCode
-- using this table to store the Customer Code final amounts by date ranges
Create table #CustomerCode (StartDate datetime, EndDate datetime, CustomerCode varchar(20), FinalAmount int)
insert into #CustomerCode
select '1900-01-01','2009-02-28', '136', 20000000 union all
select '1900-01-01','2009-02-28', '34', 100000 union all
select '1900-01-01','2009-02-28', '10', 81000 union all
select '2009-03-01','2010-03-30', '136', 10000000 union all
select '2009-03-01','2010-03-30', '34', 50000 union all
select '2009-03-01','2010-03-30', '10', 40500 union all
select '2010-03-31','9999-12-31', '136', 20000000 union all
select '2010-03-31','9999-12-31', '34', 100000 union all
select '2010-03-31','9999-12-31', '10', 81000
SELECT * FROM #TempResults
update t
set Amount_Latest = c.FinalAmount
from #TempResults t inner join #CustomerCode c
on t.CustomerCode = c.CustomerCode
and t.RecordDate between c.StartDate and c.EndDate
select * from #TempResults
For better, quicker answers, click on the following...
http://www.sqlservercentral.com/articles/Best+Practices/61537/
For better answers on performance questions, click on the following...
http://www.sqlservercentral.com/articles/SQLServerCentral/66909/
August 25, 2010 at 3:34 am
hi Mike
Thanks for that.
The actual function is a lot more involved and ive just simplified for demonstation purposes. The customercodes arent actually hardcoded in the function.
in my previous post, i meant to say Cross APPLY and not Cross JOIN
The ideal solution im looking for must include the Function call
Thanks again. Ron.
August 26, 2010 at 6:41 am
I am pretty certain that this entire process can be made SET based without a UDF (killer) nor the looping (extra killer), but it will take more time that a free forum post is worth to solve. If this is a critical process, simply hire a pro to do the refactor for you and explain how it works. OUTPUT table(s) will be key.
Oh, stop doing COUNT(*) when you really just need EXISTS. That can be SEVERELY non-performant.
Best,
Kevin G. Boles
SQL Server Consultant
SQL MVP 2007-2012
TheSQLGuru on googles mail service
Viewing 12 posts - 1 through 12 (of 12 total)
You must be logged in to reply to this topic. Login to reply