Optimize this sql 2005 query

I'm still working in SQL 2005.
This query runs too long and I need to optimize it. Can anyone help me with it?

I have attached the table and the table variable declaration.

select 
min(cd.yyyymmdd) as yyyymmdd, max(cd.id) as id, isnull(sum(cd.amount),0) as amount, isnull(sum(cd2.amount),0) as comp
from @temp_Stores sg 
   join calculationdetail cd on sg.store = cd.store and cd.id = @vCalc
   left join calculationdetail cd2 on sg.store = cd2.store and dateadd(d, -364, Convert(datetime, cd.YYYYMMDD)) = Convert(datetime,cd2.YYYYMMDD) and cd.id = cd2.id
where cd.yyyymmdd between @vBegDate and @vEndDate 
and (@Period != 'ytd' or datepart(dayofyear, Convert(datetime,cd.YYYYMMDD)) <= datepart(dayofyear, @vEndDate))
and (@Period != 'mtd' or datepart(day, Convert(datetime,cd.YYYYMMDD)) <= datepart(day, @vEndDate))
and (@Period != 'wtd' or datepart(weekday, Convert(datetime,cd.YYYYMMDD)) <= datepart(weekday, @vEndDate))
group by Case @Period 
   when 'ytd' then datepart(month, Convert(datetime,cd.YYYYMMDD)) 
   when 'mtd' then datepart(month, Convert(datetime,cd.YYYYMMDD)) 
   when 'wtd' Then datepart(week, Convert(datetime,cd.[YYYYMMDD])) END

Open in new window

Tables.sql
dgerlerAsked:
Who is Participating?
 
Steve WalesSenior Database AdministratorCommented:
I'm by no means a master at SQL Server query tuning but ...

I see a couple of things here that could be causing you problems.  Without seeing the Execution Plan, it's hard to know for sure.

You don't say how large the tables are and you're joining on non indexes columns (join conditions are on store but indexes are on id).

I believe that this is causing you to have to do scans on the tables for each join since the optimizer doesn't have indexes to seek on for the join conditions - if the tables are large, that could cause significant overhead.

I'd think that is a major contributing factor to your performance issue - do you have an Execution Plan you can show ?

I seem to recall reading somewhere once that table variables also aren't necessarily the fastest thing around either, but I can't find that reference at the moment.

EDIT: Found it!  The following blog from the MS Customer Service team (again, for large numbers of rows) mentions how performance can suffer when using table variables for large numbers of rows.

http://blogs.msdn.com/b/psssql/archive/2010/08/24/query-performance-and-table-variables.aspx
0
 
PortletPaulfreelancerCommented:
could you also provide all of the variables - as you use them
I think we need to see how you are declaring them and what typical values they hold.

Also, while I'm here why are you storing dates as [YYYYMMDD] [varchar](8) ?

do you an execution plan (.sqlplan file) you can attach?
0
 
dgerlerAuthor Commented:
Dates stored as YYYYMMDD as a Legacy that I can't get away from right now.

The number of stores is between 1 and 100 depending on the selection of the user.

The full Stored Procedure...
/****** Object:  StoredProcedure [dbo].[spGet_PeriodToDate_Calculation]    Script Date: 10/22/2013 23:03:57 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER PROCEDURE [dbo].[spGet_PeriodToDate_Calculation] 
	@vStore varchar(50)
	, @vBegDate varchar(8)
	, @vEndDate varchar(8)
	, @vCalc varchar(20)
	, @Period varchar(20)
	, @CompStores varchar(20)
AS
SET NOCOUNT ON
DECLARE @temp_Stores TABLE ( temp_ID int IDENTITY(1,1) NOT NULL PRIMARY KEY
	, Store int)

IF ISNUMERIC(@vStore) = 0 
    BEGIN
	/* All Stores or store group */
	IF LOWER(@vStore) = 'all stores'
	    BEGIN
		/* Get list of all store numbers */
		INSERT @temp_Stores
			SELECT s.[store] FROM [Stores] s left join calculationdetail cd on s.store = cd.store and cd.id = 'net1'
				where (@compStores != '1' or dateadd(d, -364, Convert(datetime, @vEndDate)) = Convert(datetime,cd.YYYYMMDD))
				and (@compStores != '0' or Convert(datetime, @vEndDate) = Convert(datetime,cd.YYYYMMDD))
	    END
	ELSE
	    BEGIN
		/* Get list of store numbers for store group */
		INSERT @temp_Stores
			SELECT sg.[store] FROM [StoreGroups] sg left join calculationdetail cd on sg.store = cd.store and cd.id = 'net1'
				where [groupname] = @vStore
				and (@compStores != '1' or dateadd(d, -364, Convert(datetime, @vEndDate)) = Convert(datetime,cd.YYYYMMDD))
				and (@compStores != '0' or Convert(datetime, @vEndDate) = Convert(datetime,cd.YYYYMMDD))
	    END
    END
ELSE
    BEGIN
	/* add individual store into table */
	INSERT @temp_Stores
		SELECT @vStore
    END

--set statistics time on
select 
min(cd.yyyymmdd) as yyyymmdd, max(cd.id) as id, isnull(sum(cd.amount),0) as amount, isnull(sum(cd2.amount),0) as comp
from @temp_Stores sg 
	join calculationdetail cd on sg.store = cd.store and cd.id = @vCalc
	left join calculationdetail cd2 on sg.store = cd2.store and dateadd(d, -364, Convert(datetime, cd.YYYYMMDD)) = Convert(datetime,cd2.YYYYMMDD) and cd.id = cd2.id
where cd.yyyymmdd between @vBegDate and @vEndDate 
and (@Period != 'ytd' or datepart(dayofyear, Convert(datetime,cd.YYYYMMDD)) <= datepart(dayofyear, @vEndDate))
and (@Period != 'mtd' or datepart(day, Convert(datetime,cd.YYYYMMDD)) <= datepart(day, @vEndDate))
and (@Period != 'wtd' or datepart(weekday, Convert(datetime,cd.YYYYMMDD)) <= datepart(weekday, @vEndDate))
group by Case @Period 
	when 'ytd' then datepart(month, Convert(datetime,cd.YYYYMMDD)) 
	when 'mtd' then datepart(month, Convert(datetime,cd.YYYYMMDD)) 
	when 'wtd' Then datepart(week, Convert(datetime,cd.[YYYYMMDD])) END

Open in new window

GetPeriod.sqlplan
0
Keep up with what's happening at Experts Exchange!

Sign up to receive Decoded, a new monthly digest with product updates, feature release info, continuing education opportunities, and more.

 
dgerlerAuthor Commented:
Looking at the execution plan, my problem is in determining the compStores rather than the bottom query.
0
 
PortletPaulfreelancerCommented:
looks to me like the execution plan only captured the top half, so that result would be expected (for that part).
0
 
PortletPaulfreelancerCommented:
I'm going to propose a small change that should reduce the number of conversions to/from varchar/datetime:
 LEFT JOIN calculationdetail cd2
        ON sg.store = cd2.store
       AND cd.id = cd2.id
       AND cd2.YYYYMMDD = convert(varchar(8),dateadd(d, -364, Convert(datetime, cd.YYYYMMDD, 112)),112)

Open in new window

this leaves on side of a join condition unchanged but forces the other half back to a varchar for direct comparison as varchar.
(instead of converting both sides to datetime)

I would suggest that approach be adopted wherever it can be.

I also think sjwales has it right regarding indexes and that the joining fields need indexing, which I expect would include that YYYMMDD field.

If you can get the rest of the execution plan it may still be useful.
0
 
dgerlerAuthor Commented:
Thanks Jimhorn.

Attached the 2nd query execution plan after making the change suggested by PortletPaul.
GetPeriod2.sqlplan
TimeStatistics.rpt
0
 
dgerlerAuthor Commented:
Also, these indexes are already on the table.


CREATE CLUSTERED INDEX [idx_YYYYMMDD] ON [dbo].[CalculationDetail]
(
      [YYYYMMDD] ASC
)
WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, SORT_IN_TEMPDB = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY]


CREATE UNIQUE NONCLUSTERED INDEX [idx_CalculationDetail4] ON [dbo].[CalculationDetail]
(
      [YYYYMMDD] ASC,
      [Store] ASC,
      [ID] ASC,
      [Report] ASC
)
INCLUDE ( [Client], [Amount])
WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, SORT_IN_TEMPDB = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY]
0
 
Scott PletcherSenior DBACommented:
You should also cluster the @temp_Stores table on the actual key, Store, and not an identity column.

DECLARE @temp_Stores TABLE ( Store int PRIMARY KEY )

If necessary, make sure you don't insert duplicate values when loading @temp_Stores.

[Btw, worst performance myth of all time: no matter what some "experts" say, an identity column should NEVER be considered a "default" clustering key.  The clustering key should istead always be carefully chosen for that specific table's data, NOT based on some nursey-rhyme-like saying about "identity always makes a good default clustering key".]
0
 
dgerlerAuthor Commented:
OK. Those steps helped the query significantly, but it still takes to long.
This query against 85 stores for Year To Date sales went from taking 71 seconds to 8.6 seconds. I appreciate your help with this.

I'd like it to be much better still.

I have attached the stored procedure as it is (old code commented out) and the execution plan.
GetPeriod3.sqlplan
GetPeriod4.sqlplan
0
 
PortletPaulfreelancerCommented:
you missed attaching the new code, but it's better if you just add it as a code block, click Code in the toolbar and paste between the 2 tags.
0
 
PortletPaulfreelancerCommented:
there are a set of unnecessary conversions to datetime in the where & group clauses, try this as a replacement:
where cd.yyyymmdd between @vBegDate and @vEndDate 
and (@Period != 'ytd' or datepart(dayofyear, cd.YYYYMMDD) <= datepart(dayofyear, @vEndDate))
and (@Period != 'mtd' or datepart(day,       cd.YYYYMMDD) <= datepart(day,       @vEndDate))
and (@Period != 'wtd' or datepart(weekday,   cd.YYYYMMDD) <= datepart(weekday,   @vEndDate))
group by Case @Period 
 when 'ytd' then datepart(month, cd.YYYYMMDD) 
 when 'mtd' then datepart(month, cd.YYYYMMDD) 
 when 'wtd' Then datepart(week,  cd.YYYYMMDD)

Open in new window

note in the exising code you are drawing datepart directly from the varchar variable @vEndDate; but forcing a convert to datetime on the varchar field YYYMMDD before each datepart. It appears that datepart will accept 'YYYYMMDD' directly.

I'd only expect a marginal improvement by this however.
0
 
dgerlerAuthor Commented:
Sorry about the code.
Here it is as it was when sending that.

/****** Object:  StoredProcedure [dbo].[spGet_PeriodToDate_Calculation]    Script Date: 10/23/2013 00:20:36 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER PROCEDURE [dbo].[spGet_PeriodToDate_Calculation] 
	@vStore varchar(50)
	, @vBegDate varchar(8)
	, @vEndDate varchar(8)
	, @vCalc varchar(20)
	, @Period varchar(20)
	, @CompStores varchar(20)
AS
SET NOCOUNT ON
DECLARE @temp_Stores TABLE ( temp_ID int IDENTITY(1,1) NOT NULL PRIMARY KEY, Store int)

IF ISNUMERIC(@vStore) = 0 
    BEGIN
	/* All Stores or store group */
	IF LOWER(@vStore) = 'all stores'
	    BEGIN
		/* Get list of all store numbers */
		INSERT @temp_Stores
			SELECT s.[store] FROM [Stores] s left join calculationdetail cd on s.store = cd.store and cd.id = 'net1'
				where (@compStores != '1' or dateadd(d, -364, Convert(datetime, @vEndDate)) = Convert(datetime,cd.YYYYMMDD))
				and (@compStores != '0' or Convert(datetime, @vEndDate) = Convert(datetime,cd.YYYYMMDD))
	    END
	ELSE
	    BEGIN
		/* Get list of store numbers for store group */
		INSERT @temp_Stores
			SELECT sg.[store] FROM [StoreGroups] sg left join calculationdetail cd on sg.store = cd.store and cd.id = 'net1'
				where [groupname] = @vStore
				and (@compStores != '1' or dateadd(d, -364, Convert(datetime, @vEndDate)) = Convert(datetime,cd.YYYYMMDD))
				and (@compStores != '0' or Convert(datetime, @vEndDate) = Convert(datetime,cd.YYYYMMDD))
	    END
    END
ELSE
    BEGIN
	/* add individual store into table */
	INSERT @temp_Stores
		SELECT @vStore
    END

--set statistics time on
select 
min(cd.yyyymmdd) as yyyymmdd, max(cd.id) as id, isnull(sum(cd.amount),0) as amount, isnull(sum(cd2.amount),0) as comp
from @temp_Stores sg 
	join calculationdetail cd on sg.store = cd.store and cd.id = @vCalc
	--left join calculationdetail cd2 on sg.store = cd2.store and dateadd(d, -364, Convert(datetime, cd.YYYYMMDD)) = Convert(datetime,cd2.YYYYMMDD) and cd.id = cd2.id
	LEFT JOIN calculationdetail cd2
        ON sg.store = cd2.store
       AND cd.id = cd2.id
       AND cd2.YYYYMMDD = convert(varchar(8),dateadd(d, -364, Convert(datetime, cd.YYYYMMDD, 112)),112)
                                          
where cd.yyyymmdd between @vBegDate and @vEndDate 
and (@Period != 'ytd' or datepart(dayofyear, Convert(datetime,cd.YYYYMMDD)) <= datepart(dayofyear, @vEndDate))
and (@Period != 'mtd' or datepart(day, Convert(datetime,cd.YYYYMMDD)) <= datepart(day, @vEndDate))
and (@Period != 'wtd' or datepart(weekday, Convert(datetime,cd.YYYYMMDD)) <= datepart(weekday, @vEndDate))
group by Case @Period 
	when 'ytd' then datepart(month, Convert(datetime,cd.YYYYMMDD)) 
	when 'mtd' then datepart(month, Convert(datetime,cd.YYYYMMDD)) 
	when 'wtd' Then datepart(week, Convert(datetime,cd.YYYYMMDD)) END

Open in new window

0
 
dgerlerAuthor Commented:
Would the you say same unnecessary conversion is happening in lines 26 and 35 of the store selection section?
0
 
PortletPaulfreelancerCommented:
>>Would the you say same unnecessary conversion is happening in lines 26 and 35 of the store selection section?
Yes

Try to minimize all conversions, in both those cases manipulate the variable back to varchar and leave the data unconverted. Look for all such opportunities.

In essence aim for "sargable" predicates:
http://en.wikipedia.org/wiki/Sargable

Rules of thumb
Avoid using functions on row values in a sql condition.
Avoid non-sargable predicates and replace them with sargable equivalents.
0
 
dgerlerAuthor Commented:
Those changes helped marginally as you predicated.

I'm looking at the information about "sargable" predicates now.
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

All Courses

From novice to tech pro — start learning today.