March 17, 2016 at 8:37 am
Hi,
I have created this SP and I am wondering if anyone can help me optimise the sp and its taking too long to run
March 17, 2016 at 8:54 am
When posting optimization questions, it really helps to provide the actual execution plan (or at least the estimated execution plan).
That being said, the first thing I see is an overuse of temp tables, multiple updates to the same table, and multiple queries on the same table.
For example, you have the following code:
SELECT s.ProductCode,
min(Date) AS MinDate,
min(s.Period) AS MinPeriod
INTO #MINPeriod
FROM [cbis799p].[dbo].[SHIPP_DIVPROD_WEEK] s
INNER JOIN cbis799p.dbo.PRODUCT ON s.ProductCode = PRODUCT.ProductCode
LEFT JOIN Datawarehouse.dbo.Calendar ON Calendar.WeeklyPeriod = s.Period
WHERE Retail > 0
AND Calendar.Datetime >= @StartOfWeek
AND Calendar.Datetime <= @EndOfWeek
AND Divno like @Country
AND ProductClass = 1
GROUP BY s.ProductCode
SELECT s.ProductCode,
max(Date) AS MaxDate,
max(s.Period) AS MaxPeriod
INTO #MAXPeriod
FROM [cbis799p].[dbo].[SHIPP_DIVPROD_WEEK] s
INNER JOIN cbis799p.dbo.PRODUCT ON s.ProductCode = PRODUCT.ProductCode
LEFT JOIN Datawarehouse.dbo.Calendar ON Calendar.WeeklyPeriod = s.Period
WHERE Retail > 0
AND Calendar.Datetime >= @StartOfWeek
AND Calendar.Datetime <= @EndOfWeek
AND Divno like @Country
AND ProductClass=1
GROUP BY s.ProductCode
Which can easily be combined to roughly cut the cost of this particular section in half.
SELECT s.ProductCode,
min(Date) AS MinDate,
min(s.Period) AS MinPeriod,
max(Date) AS MaxDate,
max(s.Period) AS MaxPeriod
INTO #Period
FROM [cbis799p].[dbo].[SHIPP_DIVPROD_WEEK] s
INNER JOIN cbis799p.dbo.PRODUCT ON s.ProductCode = PRODUCT.ProductCode
LEFT JOIN Datawarehouse.dbo.Calendar ON Calendar.WeeklyPeriod = s.Period
WHERE Retail > 0
AND Calendar.Datetime >= @StartOfWeek
AND Calendar.Datetime <= @EndOfWeek
AND Divno like @Country
AND ProductClass = 1
GROUP BY s.ProductCode
So you use two temp tables when you can use one and you read the source tables twice when you only need to read them once.
Drew
J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA
March 17, 2016 at 9:02 am
You're using temp tables to store results that are only used once(#MinPeriod and #MaxPeriod). It is better to use a CTE or a derived table for single use temporary results.
Drew
J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA
March 17, 2016 at 9:10 am
If you want more accurate help, read this: http://www.sqlservercentral.com/articles/SQLServerCentral/66909/
That's a long procedure and optimizing it could take days, you won't get full help from forum posts, so either you break it up or hire a consultant that will fix it and teach you how to fix similar problems.
March 17, 2016 at 9:15 am
Thanks for the reply guys, I'll use CTE's instead and break it down further.
Regards
March 17, 2016 at 10:07 am
1) There are often times when using a temp table (NOT table variable!!) can make a process be WAY more efficient due to widely varying amounts of rows or key values within those rows.
2) This stuff should almost certainly have OPTION (RECOMPILE) on it. I recommend that for virtually any query that has a start/end date pair, especially complex ones. You NEVER EVER want plan caching in any form for such queries. Think of compiling a query for a very efficient index seek/nested loop join plan for 1 hour or one 1 day of date range. Now that exact same query plan gets used for @Start = '1900-01-01', @End = '9999-12-31'. Do you REALLY want to use the same plan for all data?? You get killed the opposite direction too.
This type of thing makes me look like I walk on water when I show clients the difference in performance (and concurrency too often!). 😎
Best,
Kevin G. Boles
SQL Server Consultant
SQL MVP 2007-2012
TheSQLGuru on googles mail service
March 17, 2016 at 12:51 pm
This looks a bit heavy
AND Calendar.Datetime >= @StartOfWeek
AND Calendar.Datetime <= @EndOfWeek
Can you add a column to Calendar that can make it
AND Calendar.WeekYear = @WeekYear
March 17, 2016 at 1:00 pm
The Danish Dynamo (3/17/2016)
This looks a bit heavyAND Calendar.Datetime >= @StartOfWeek
AND Calendar.Datetime <= @EndOfWeek
Can you add a column to Calendar that can make it
AND Calendar.WeekYear = @WeekYear
What do you mean by heavy?
If Datetime is the clustered index, changing to weekyear would require an additional index.
March 17, 2016 at 2:28 pm
The person has a calendar where it can add calculated values, if you think CPU then date range is slower than comparing to a small int.
Is there a function f(@StartOfWeek,@EndOfWeek) => Calendar.WeeklyPeriod
March 17, 2016 at 2:39 pm
The Danish Dynamo (3/17/2016)
The person has a calendar where it can add calculated values, if you think CPU then date range is slower than comparing to a small int.Is there a function f(@StartOfWeek,@EndOfWeek) => Calendar.WeeklyPeriod
And what if the week is a rolling period of 7 days? One run may be Moday through the following Sunday, the next run may be Tuesday through the following Monday.
We don't the requirements as that information was not provided.
March 17, 2016 at 7:08 pm
TheSQLGuru (3/17/2016)
1) using a temp table (NOT table variable!!)
Not sure if there is much of a difference here.
Scoping - yes, quite different.
Can you name anything else?
_____________
Code for TallyGenerator
March 18, 2016 at 2:52 am
Sergiy (3/17/2016)
Can you name anything else?
Statistics mostly, table variables don't have them. Lots of rows in a table variable can result in really bad row count estimates by the optimiser (mix of no stats and no recompiles) and resultant 'not great' plans.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
March 18, 2016 at 9:35 am
Sergiy (3/17/2016)
TheSQLGuru (3/17/2016)
1) using a temp table (NOT table variable!!)Not sure if there is much of a difference here.
Scoping - yes, quite different.
Can you name anything else?
You can create a non-unique clustered index; that can be very valuable at times.
Temp tables do cause more recompilations, but that can be a good thing when it generates a better query plan after rows are added.
SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.
March 18, 2016 at 11:53 am
GilaMonster (3/18/2016)
Sergiy (3/17/2016)
Can you name anything else?Statistics mostly, table variables don't have them. Lots of rows in a table variable can result in really bad row count estimates by the optimiser (mix of no stats and no recompiles) and resultant 'not great' plans.
It is more than just rowcounts too. I have a very simple AdventureWorks-based demo in my Common TSQL Mistakes session where a SINGLE-row, SINGLE-column table variable gets you a bad plan some of the time but a temp table gets you the optimal plan every time. Data value skew can be a CRUSHINGLY BAD thing if you don't deal with it 100% of the time. Almost every client I encounter has that to varying degrees.
I have lost track of the number of cases where I have replaced table vars with temp tables for incredible wins at clients. I could probably count on one hand the number of times I have done the opposite (and some of those will be the esoteric edge cases where you should use a table var over a temp table).
Best,
Kevin G. Boles
SQL Server Consultant
SQL MVP 2007-2012
TheSQLGuru on googles mail service
Viewing 14 posts - 1 through 13 (of 13 total)
You must be logged in to reply to this topic. Login to reply