June 11, 2019 at 10:23 am
Hi all,
I've been trying to optimize a large stored procedure. I found a code there that insert records in a temp table doing a loop. The code is something like this:
DECLARE @FDATO INT = 79627
DECLARE @TDATO INT = 79657
DECLARE @DATO INT
DECLARE @TIME DATETIME = GETDATE()
SET NOCOUNT ON
IF OBJECT_ID('tempdb..#STDORDDATO') IS NOT NULL
DROP TABLE #STDORDDATO
create table #STDORDDATO (Prec int, Dato int)
set @dato = @fdato
while @dato <= @tdato
begin
insert into #STDORDDATO (Prec, Dato)
select std.prec, @dato
from dbo.stdord sto
inner join dbo.STDORDRE std on std.KUNDE = sto.KUNDE and std.DATO = sto.DATO
inner join dbo.DBVARE var on var.NR = std.VARENR
CROSS APPLY (SELECT MAX(DATO) AS dato
FROM dbo.STDORD
WHERE DATO <= @dato AND KUNDE = sto.KUNDE AND ISNULL(ANN, 0) <> 1) CA
where sto.dato = ca.dato and sto.TYPE = 1
and var.TYPE < 5
and (datepart(dw, dbo.MCS_ClarionDateToSQL(@dato)) = 2 and (ISNULL(STD.D1, '') <> '' or ISNULL(STD.BD1, '') <> '')
or datepart(dw, dbo.MCS_ClarionDateToSQL(@dato)) = 3 and (ISNULL(STD.D2, '') <> '' or ISNULL(STD.BD2, '') <> '')
or datepart(dw, dbo.MCS_ClarionDateToSQL(@dato)) = 4 and (ISNULL(STD.D3, '') <> '' or ISNULL(STD.BD3, '') <> '')
or datepart(dw, dbo.MCS_ClarionDateToSQL(@dato)) = 5 and (ISNULL(STD.D4, '') <> '' or ISNULL(STD.BD4, '') <> '')
or datepart(dw, dbo.MCS_ClarionDateToSQL(@dato)) = 6 and (ISNULL(STD.D5, '') <> '' or ISNULL(STD.BD5, '') <> '')
or datepart(dw, dbo.MCS_ClarionDateToSQL(@dato)) = 7 and (ISNULL(STD.D6, '') <> '' or ISNULL(STD.BD6, '') <> '')
or datepart(dw, dbo.MCS_ClarionDateToSQL(@dato)) = 1 and (ISNULL(STD.D7, '') <> '' or ISNULL(STD.BD7, '') <> ''))
set @dato = @dato + 1
END
I don't like the fact to be inserting row by row so I tried a couple of things. First, with a CTE that generates the sequence of numbers and then crossing apply that CTE. Code is this:
IF OBJECT_ID('tempdb..#STDORDDATO2') IS NOT NULL
DROP TABLE #STDORDDATO2
create table #STDORDDATO2 (Prec int, Dato int)
;With DateSequence as
(
SELECT @FDATO [n]
UNION all
SELECT n + 1
FROM DateSequence
WHERE n < @TDATO
)
--SELECT * FROM DateSequence
INSERT INTO #STDORDDATO2 ( Prec ,
Dato )
SELECT std.prec, ds.n
FROM dbo.stdord sto
inner join dbo.STDORDRE std on std.KUNDE = sto.KUNDE and std.DATO = sto.DATO
inner join dbo.DBVARE var on var.NR = std.VARENR
CROSS APPLY (SELECT n FROM DateSequence) DS
CROSS APPLY (SELECT MAX(DATO) AS dato
FROM dbo.STDORD
WHERE DATO <= DS.n AND KUNDE = sto.KUNDE AND ISNULL(ANN, 0) <> 1) CA
WHERE 1 = 1
AND sto.dato = ca.dato
AND sto.TYPE = 1
AND var.TYPE < 5
and (datepart(dw, dbo.MCS_ClarionDateToSQL(ds.N)) = 2 and (ISNULL(STD.D1, '') <> '' or ISNULL(STD.BD1, '') <> '')
or datepart(dw, dbo.MCS_ClarionDateToSQL(ds.N)) = 4 and (ISNULL(STD.D3, '') <> '' or ISNULL(STD.BD3, '') <> '')
or datepart(dw, dbo.MCS_ClarionDateToSQL(ds.N)) = 5 and (ISNULL(STD.D4, '') <> '' or ISNULL(STD.BD4, '') <> '')
or datepart(dw, dbo.MCS_ClarionDateToSQL(ds.N)) = 6 and (ISNULL(STD.D5, '') <> '' or ISNULL(STD.BD5, '') <> '')
or datepart(dw, dbo.MCS_ClarionDateToSQL(ds.N)) = 7 and (ISNULL(STD.D6, '') <> '' or ISNULL(STD.BD6, '') <> '')
or datepart(dw, dbo.MCS_ClarionDateToSQL(ds.N)) = 1 and (ISNULL(STD.D7, '') <> '' or ISNULL(STD.BD7, '') <> '')
OR datepart(dw, dbo.MCS_ClarionDateToSQL(ds.N)) = 3 and (ISNULL(STD.D2, '') <> '' or ISNULL(STD.BD2, '') <> ''))
Finally, I use a function called GetNums that returns a list of numbers from 1 to a parameter. Result query is similar to the previous one.
I was surprised when I see execution time:
Execution time 1: 150 ms
Execution time 2: 1123 ms
Execution time 3: 1320 ms
So, according to this, looping and inserting row by row is faster than the other 2 options. Is there a better way to do it? Just curious.
Thanks in advance.
June 11, 2019 at 7:33 pm
There are a couple of things we need to be able to help you.
Without those, all I can say is that, like many folks, you may have not joined the "GetNums" table correctly and that's the likely problem. It's a common problem with a lot of folks.
--Jeff Moden
June 13, 2019 at 12:42 pm
Thanks for your answer, Jeff. I've already solved the problem.
June 13, 2019 at 1:44 pm
Thanks for your answer, Jeff. I've already solved the problem.
If you ended up using some other than a "GetNums" or other "Tally-like" function, you still have a problem, provided THAT was written correctly to begin with.
So my question is, how did you solve the problem and what did you end up using? Just tryin' to help here.
--Jeff Moden
June 13, 2019 at 1:48 pm
I've used GetNums, which is like this:
ALTER FUNCTION [dbo].[GetNums](@n AS BIGINT) RETURNS TABLE AS RETURN
WITH
L0 AS(SELECT 1 AS c UNION ALL SELECT 1),
L1 AS(SELECT 1 AS c FROM L0 AS A CROSS JOIN L0 AS B),
L2 AS(SELECT 1 AS c FROM L1 AS A CROSS JOIN L1 AS B),
L3 AS(SELECT 1 AS c FROM L2 AS A CROSS JOIN L2 AS B),
L4 AS(SELECT 1 AS c FROM L3 AS A CROSS JOIN L3 AS B),
L5 AS(SELECT 1 AS c FROM L4 AS A CROSS JOIN L4 AS B),
Nums AS(SELECT ROW_NUMBER() OVER(ORDER BY (SELECT NULL)) AS n FROM L5)
SELECT TOP (@n) n FROM Nums ORDER BY n;
Function MCS_ClarionDateToSQL is just a scalar function:
ALTER FUNCTION [dbo].[MCS_ClarionDateToSQL]
(
@pDate INT
)
RETURNS DATETIME
AS
BEGIN
DECLARE @Result DATETIME;
SELECT @Result = DATEADD(DAY, @pDate - 4, '1801-01-01');
RETURN @Result;
END;
Do you think GetNums is wrong?
Thanks for your help!
June 13, 2019 at 3:06 pm
Function MCS_ClarionDateToSQL is just a scalar function:
Do you think GetNums is wrong? Thanks for your help!
Yes... it's partially incorrect. It has a real ORDER BY in it. Since it's contained in a function, that ORDER BY constitutes a "Nested Order By" and that can cause performance issues much like it does in Views because the iTVF (inline Table Valued Function) is also known as an "Inline View" for a reason.
Getting back to the subject at hand, your original post didn't include the code for how you tried to solve the problem using the GetNums function. Since you posted that it was the worst performing, I'd like to see what you've done there. You've also not posted what you finally settled on for your final solution.
I'll also state that both pieces of code seem to have issues with the WHERE clause and, perhaps, with how the WHERE clause in the CROSS APPLY (missing table aliases can be a really problem there).
There's also a wealth of "anti-performance" sins in the where clause such as "and (ISNULL(STD.D1, '') <> ''" which can easily be replaced by "and STD.D1 > ''" simply because of the way NULLs actually work.
So, bottom line is the same as before...
Heh... or not and we'll understand that your "sufficiency has been suffonsified". 🙂 Again, just tryin' to help here.
--Jeff Moden
June 13, 2019 at 3:09 pm
Mauricio_ wrote:Function MCS_ClarionDateToSQL is just a scalar function: Do you think GetNums is wrong? Thanks for your help!
Yes... it's partially incorrect. It has a real ORDER BY in it. Since it's contained in a function, that ORDER BY constitutes a "Nested Order By" and that can cause performance issues much like it does in Views because the iTVF (inline Table Valued Function) is also known as an "Inline View" for a reason. Getting back to the subject at hand, your original post didn't include the code for how you tried to solve the problem using the GetNums function. Since you posted that it was the worst performing, I'd like to see what you've done there. You've also not posted what you finally settled on for your final solution. I'll also state that both pieces of code seem to have issues with the WHERE clause and, perhaps, with how the WHERE clause in the CROSS APPLY (missing table aliases can be a really problem there). There's also a wealth of "anti-performance" sins in the where clause such as "and (ISNULL(STD.D1, '') <> ''" which can easily be replaced by "and STD.D1 > ''" simply because of the way NULLs actually work. So, bottom line is the same as before...
- Please Post the missing 3rd example code where the "GetNums" table was used and lost.
- Since we have absolutely zero of the tables you're JOINing to, you need to post a better description of what the code is supposed to do and provide some sample output.
- And I'll also add, please post the code you used to "solve the problem.
Heh... or not and we'll understand that your "sufficiency has been suffonsified and anything additional would be superfluous". 🙂 Again, just tryin' to help here.
--Jeff Moden
June 13, 2019 at 3:27 pm
Why not just create a permanent Tally table? It's probably be useful in other situations. Just wondering...
--Vadim R.
June 13, 2019 at 3:42 pm
Why not just create a permanent Tally table? It's probably be useful in other situations. Just wondering...
I actually have both a Tally table and an iTFV that works like a Tally table (fnTally).
The Tally table has a performance edge over the iTVF and I use it for a wealth of things but it does have a disadvantage when it comes to the number of logical reads. In this case, it's not a bad thing but some folks are bothered by a high number of logical reads and rightly so in some cases.
The iTFV produces zero logical reads and that can be a huge advantage to some.
As with all else in SQL Server, "It Depends" but I have and use both.
--Jeff Moden
June 13, 2019 at 4:40 pm
Got it. Thanks Jeff.
--Vadim R.
Viewing 10 posts - 1 through 9 (of 9 total)
You must be logged in to reply to this topic. Login to reply