November 25, 2008 at 9:36 am
I am working with a developer and he is having trouble with his stored procedure that he is developing. I am pretty new to the DBA role and have minimal coding background. Here is his code:
DECLARE @PeriodFrom char(5)
DECLARE @PeriodTo char(5)
DECLARE @Property varchar(2000)
DECLARE @db varchar(50)
DECLARE @ConcessionCode varchar(2000)
SET @PERIODFROM = '09/08'
SET @PeriodTo = '11/08'
SET @db = 'MRI_PROD'
SET @CONCESSIONCODE='C01,C02,C03,C04,C05,C06,C06,C07,C08'
SET @Property='493,495,547,556,560,561,562,563,566,567,570,584,644,645,696,
711,712,715,720,721,723,669,679,691,705,706,708,709,710,716,692,693,230,231,
510,672,673,674,675,676,519,596,631,632,649,245,255,633,646,648,657,659,661,
662,663,665,666,667,699,514,680,254,504,505,509,512,513,516,516G,518,685,686,
687,697,698,717,241,266,271,274,502,523,525,541,542,543,545,546,557,606,606G,
607,608,609,610,611,653,700,427,428,537,552,553,565,572,623,636,647,651,670,677,
678,681,682,690,695,701,702'
Declare @sql Varchar(8000)
DECLARE @PeriodList varchar(20)
DECLARE @StartMonth char(2)
DECLARE @EndMonth char(2)
DECLARE @StartYear char(4)
DECLARE @EndYear char(4)
DECLARE @counter int
DECLARE @DateStart datetime
DECLARE @DateEnd datetime
DECLARE @CurMonth datetime
DECLARE @tblPeriods table(Period varchar(12))
SET @StartMonth = (SELECT LEFT(@PeriodFrom,2))
SET @StartYear = (SELECT '20' + RIGHT(@PeriodFrom,2))
SET @EndMonth = (SELECT LEFT(@PeriodTo,2))
SET @EndYear = (SELECT '20' + RIGHT(@PeriodTo,2))
SET @DateStart = CAST((SELECT @StartYear + '-' + @StartMonth + '-01') as datetime)
SET @DateEnd = CAST((SELECT @EndYear + '-' + @EndMonth + '-01') as datetime)
IF @DateStart > @DateEnd BEGIN SET @DateStart = @DateEnd END
SET @CurMonth = + @DateStart
while @CurMonth <= @DateEnd
BEGIN
SET @PeriodList = (SELECT cast(YEAR(@CurMonth) as varchar) + CASE LEN(MONTH(@CurMonth))
When 1 Then '0' + CAST(MONTH(@CurMonth) as varchar)
ELSE CAST(MONTH(@CurMonth) as varchar)
END)
INSERT INTO @tblPeriods(Period) VALUES(@PeriodList)
SET @CurMonth = DATEADD(month, 1, @CurMonth)
END
select rtrim(m.mngrname) as RPM, rtrim(p.propname) as [Property Name],
rtrim(n.firstname) + ' ' + rtrim(n.lastname) as [Resident Name],
rtrim(n.rmpropid)+ '-' + rtrim(n.rmbldgid) + '-' + rtrim(n.unitid) + '-' + cast(n.rmlease as varchar) as [Resident ID],
coalesce(l.curtermstart,l.occdate) as [Resident Lease Start Date],
l.leasnm as [Lease Term (months)],
r.period as [Bill Period],
(select sum(tranamt) from MRI_PROD.dbo.rmledg r2 WITH (NOLOCK) where
r2.nameid = n.nameid
and r2.period=r.period
and chgcode='RNT'
and (r2.descrptn like 'AUTOCHRG%'
or r2.descrptn like 'Scheduler Move In%')) as [Monthly Lease Amount],
r.chgcode as [Concession Code],
c.descrptn as [Concession Description],
r.tranamt as [Amount of Concession]
from MRI_PROD.dbo.name n WITH(NOLOCK, INDEX(IDX_NAME_RESPROPUNIT, NAME_STATUS)),
MRI_PROD.dbo.rmledg r WITH(NOLOCK),
MRI_PROD.dbo.rmprop p WITH(NOLOCK),
dbo.rmmngr m WITH(NOLOCK),
MRI_PROD.dbo.rmlease l WITH(NOLOCK),
MRI_PROD.dbo.chgcode c WITH(NOLOCK)
where n.respropid = p.rmpropid
and n.rmpropid=l.rmpropid
and n.rmbldgid=l.rmbldgid
and n.unitid=l.unitid
and n.rmlease=l.rmlease
and n.nameid=r.nameid
and m.rmmngrid = p.rmmngrid
and p.RMPROPID in (Select Value from mri_dev.dbo.fn_split(@Property,','))
and r.chgcode in (Select Value from mri_dev.dbo.fn_split(@ConcessionCode,','))
and r.period in(Select * from @tblPeriods)
and n.type = 'R'
and n.status= 'C'
and r.chgcode=c.chgcode
and r.tranamt<0
order by p.propname,n.lastname,n.firstname,r.period
He has been testing and noted that most tables do not have indexes for this procedure. He has tried running without certain variables and feels that the concession codes (chgcodes) are causing the problem. Before we tackle any indexes, he wants to know if the structure of the code looks correct. Does anyone see anything that sticks out as incorrect? Like I said, I am a noob when it comes to coding and I didn't see anything wrong with this.
Any help is much appreciated!!
November 25, 2008 at 9:54 am
From having a quick look at this a few things stand out as bad performing code;
-While loop at the start, A while loop is not usualy a good idea in SQL unless absolutely neccesay. A better approach would be to build a lookup table and then use a join to obtain these values
-The code seems to contain a lot of sub-queries and these can be a source of poor performance, agian it can be better to acheive the same results with a join
-LIKE where clauses eg. r2.descrptn like 'Scheduler Move In%' will not perform as well as direct comparisons, esp without indexes.
-You are using the old method of joining tables putting the join conditions in the where clause, it is much better to use an Inner Join with the join conditions, this will not make the query faster but will make it a lot more clear as what is a Join Condition and what is a Where filter.
-It seems to rely a lot on dbo.fn_split, this could cause performance issues, it may be better again to do a join or use an IN clause.
That is all i could pick up with a quick look, have a look at your execution plan and this will give you a lot of clues as where the poor parts of the code are. It is probably better to fix the code, then build the correct indexes for opitmal speed.
November 25, 2008 at 10:09 am
I agree with all that steve has said,
I'd just like to add that I see a INDEX hint?
It's not often that the optimizer is wrong, so make sure the hint is not slowing down you query.
If sql is using the wrong index it's normally cause it doesn't have a enough info or choice.
I would also say that using the split function for a constant string seems pointless.
The function will probably have to split that string for every row that is in the query to get the where clause to work.
I would as Steve mentioned insert those into a table and join to your query
----------------------------------------------
Try to learn something about everything and everything about something. - Thomas Henry Huxley
:w00t:
Posting Best Practices[/url]
Numbers / Tally Tables[/url]
November 25, 2008 at 10:13 am
Can you please fix the wrapping?
What's the proc supposed to do?
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
Viewing 4 posts - 1 through 4 (of 4 total)
You must be logged in to reply to this topic. Login to reply