Please start any new threads on our new
site at https://forums.sqlteam.com. We've got lots of great SQL Server
experts to answer whatever question you can come up with.
Author |
Topic |
roswell
Starting Member
31 Posts |
Posted - 2007-06-26 : 14:44:55
|
CREATE PROCEDURE dbo.usp_JobsAdmin@GroupID int,@JobID nvarchar(8) output,@Client nvarchar(50) output,@Phys nvarchar(50) output,@MT nvarchar(50) output,@StartDate datetime output,@EndDate datetime output,@PatName nvarchar(50) output,@PatNo nvarchar(50) output,@Role Int,@PersonID intASDeclare @TransID intDeclare @BJobID nvarchar(8)Declare @EJobID nvarchar(8)Declare @ClientID intDeclare @BClientID intDeclare @EClientID intinsert into LogTable(msg) values ('Starting the SP')If @Client = 'NA'BeginSet @Client = '%'EndSelect @BClientID = Min(ClientID) from tblClient where client like @ClientSelect @EClientID = Max(ClientID) From tblClient where client like @ClientIf @MT <> 'NA'BeginSelect @TransID = PersonID from tblPeople where FullName = @MTEndIf @Phys = 'NA'BeginSet @Phys = '%'EndIf @MT = 'NA'BeginSet @MT = '%'EndIf @PatName = 'NA'BeginSet @PatName = '%'EndIf @PatNo = 'NA'BeginSet @PatNo = '%'EndIf @EndDate = '1/1/1800'BeginSet @EndDate = DateAdd(Day,10,GetDate())EndSet @StartDate = Convert(Char(10),@StartDate,101)Set @EndDate = Convert(Char(10),@EndDate,101)If @JobID = '0'BeginSelect @EJobID = Max(JobID) from tbl_JobSelect @BJobID = 0EndIF Right(@JobID,1) = '%'BeginSelect @EJobID = Max(JobID) from tbl_JobSet @BJobID = Left(@JobID,Len(@JobID) - 1) + '0'EndIf @JobID <> '0' and Right(@JobID,1) <> '%'BeginSet @BJObID = @JobIDSet @EJobID = @JobIDEndIf @Role = 2 Or @Role = 7Begin--If Right(@JobID,1) = '0'--Begininsert into LogTable(msg) values ('Fetching Jbos for Role 2 and 7')Select DistinctJobID,Client, ClientID, PhysID, FullName, DicID,DocExp,PriDescript,CurrentState,State,AssignedMT,GroupID,TransNo,DocTransNo,AudioTransNo,isNull(DocStart,'') as DocStart,IsNull(DocEnd,'') as DocEnd,isNull(MTName,'Not Assigned') As 'MTName',IsNull(PatientName,'None') as 'PatientName',IsNull(PatientID,'None') as 'PatientID',isNull(QAName,'Not Assigned') As 'QAName', NoAssocJob,Revision,StartDT,JobDate,Descriptionfrom vw_JobsAdminWhere (GroupID = @GroupID and ClientID >= @BClientID and ClientID <= @ECLientID And FullName Like @Phys And isNull(Convert(Char(10),JobDate,101),'') Between @StartDate And @EndDate And isNull(MTName,'NA') Like @MT And IsNull(PatientName,'NA') Like @PatName And IsNull(PatientID,'NA') Like @PatNo ) And JobID >= @BJObID and JobID <= @EJObID --and isnull(Client,'NA') Like @ClientOrder by DocExpEndGOThis SP working fine but now seems not working. this SP runs twice and I got the MsgTable entries twice. However now it runs only first time. From .NET application, it throws"Timeout expired. The timeout period elapsed prior tocompletion of the operation or the server is not responding. atSystem.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior cmdBehavior, RunBehaviorrunBehavior, Boolean returnStream)at System.Data.SqlClient.SqlCommand.ExecuteNonQuery()at tbl4.frmJobsAdmin.RealData() inD:\Projects\tblTranscription\tblTranscription\frmJobsAdmin.aspx.vb:line 3393"Then I increased the timeout to 4000 and then it throws the errorGeneral network error. Check your network documentation. atSystem.Data.SqlClient.SqlConnection.OnError(SqlException exception, TdsParserStatestate)??? I'm dumbfound here please help |
|
dinakar
Master Smack Fu Yak Hacker
2507 Posts |
Posted - 2007-06-26 : 14:51:17
|
Too many IF's can cause bad query plans. Use a CASE instead. how many records does tblclient hold? Perhaps these 2 statements:Select @BClientID = Min(ClientID) from tblClient where client like @ClientSelect @EClientID = Max(ClientID) From tblClient where client like @Client are the ones you should be looking at when @client is '%' since it will do a table scan.Dinakar Nethi************************Life is short. Enjoy it.************************http://weblogs.sqlteam.com/dinakar/ |
 |
|
roswell
Starting Member
31 Posts |
Posted - 2007-06-26 : 14:57:24
|
tblClient holds 21 rows.it was working right few weeks before....why was it working before and not now?MY DB size is 22 GB |
 |
|
dinakar
Master Smack Fu Yak Hacker
2507 Posts |
Posted - 2007-06-26 : 15:10:57
|
Why do you even need a WHERE condition in those 2 statements if @client = '%'. Use a CASE like: Select @Phys = Case WHEN @Phys = 'NA' Then '%' Else @Phys End .check the query plan. Do you see any scan's/bookmark lookups etc?Dinakar Nethi************************Life is short. Enjoy it.************************http://weblogs.sqlteam.com/dinakar/ |
 |
|
roswell
Starting Member
31 Posts |
Posted - 2007-06-27 : 05:38:23
|
Its done however the same result... |
 |
|
roswell
Starting Member
31 Posts |
Posted - 2007-06-27 : 06:49:26
|
the sp takes 4 minutes to run in query analyzer and returns 17 thousands rows |
 |
|
Kristen
Test
22859 Posts |
Posted - 2007-06-27 : 07:04:25
|
"[it was working right few weeks before....why was it working before and not now?[/i]"Probably:Table size has grown, and you are just over the timeout limit, or you are transferring more data than can be handledIndexes, or Statistics, are out of shape and are not getting rebuilt regularlyServer is busier so query taking longer, and exaserbating both of the above.So to say, but the query is really badly written from an efficiency viewpoint.The "Distinct" raises immediate alarm bells to me.This type of statement:And isNull(MTName,'NA') Like @MT is grossly inefficient.You will not get a row at all if MTName='NA' (whatever @MT is set to) - is that intended?Sounds like the database design is not brilliant anyway. Why is MTName not storing NULL for unknown, instead of the arbitrary value of "NA"?What is the typical number of results from this query? How many do you get without the DISTINCT? and how many rows in total in that table (or rather the vw_JobsAdmin view)?Kristen |
 |
|
roswell
Starting Member
31 Posts |
Posted - 2007-06-27 : 07:13:39
|
This type of statement:And isNull(MTName,'NA') Like @MTis grossly inefficient.I understand and I agree with that.Why is MTName not storing NULL for unknown, instead of the arbitrary value of "NA"? "NA" comes from the asp.net code, What is the typical number of results from this query? Seventeen thousands rowsHow many do you get without the DISTINCT?Same and how many rows in total in that table (or rather the vw_JobsAdmin view)?33661 |
 |
|
Kristen
Test
22859 Posts |
Posted - 2007-06-27 : 09:27:25
|
""NA" comes from the asp.net code"Not a good reason in my book! It should be converted to NULL as the data goes into the database - if indeed NULL is the appropriate value to use given that the data is unknown. Others may disagree.Blimey, so you are getting a timeout selecting 17,000 rows from a table with only 33,000 rows in it? That's really inefficient then, or the server is way-too-busy!, or (and perhaps most likely) the real problem is with network performance.I would do a COUNT(*) on the exact same query (i.e. same WHERE clause) and compare that with the actual SELECT. The the COUNT(*) is really quick, and getting 17,000 rows is really slow then its almost certainly a problem shipping the data to the application. Maybe the NIC is set in auto-duplex or somesuch. Or there is a duff network card somewhere and its retry-ing like mad (and the data getting through, but slower than normal).Note that if you are SURE that your query will always give the same results with & without the DISTINCT get rid of it. SQL Server is having to put all the data into a Temp table, sort it, and then compare adjacent rows in order to remove duplicates - and if there aren't any you can leave the DISTINCT out and avoid that "cost".I would recommend that you re-code the IsNull stuff. Perhaps something like change:And isNull(MTName,'NA') Like @MTtoAnd (MTName <> 'NA' AND (@MT = 'NA' OR MTName Like @MT))(don't set @MT = '%' any more). Hopefully this will optimise better.better still would be to use dynamic SQL executed via sp_ExecuteSQL for this job. This would result in multiple query plans being stored in the cache, one for each "combination" of criteria, but it should be very fast.This code:And (MTName <> 'NA' AND (@MT = 'NA' OR MTName Like @MT))would actually be:If @MT = 'NA' then "MTName <> 'NA'"else "(MTName <> 'NA' AND MTName Like @MT)"so each of those variations would be liable to be cached, with an optimal query plan for each. Whereas a Generic Query Plan (such as you are currently getting) is only ever going to be able to use a table scan Also worth having a look at the core code for the vw_JobsAdmin view and working out what indexes are in place, and whether more / different indexes would help.If that is not something you are familiar with doing I can provide some suggestions of how to find out what indexes etc. SQL Server is using, and how to analyse any improvements that could be made.Kristen |
 |
|
roswell
Starting Member
31 Posts |
Posted - 2007-06-27 : 10:50:49
|
Found the culprit statementits isNull(Convert(Char(10),JobDate,101),'') Between @StartDate And @EndDateI changed it to JobDate Between @StartDate And @EndDateand everything seems to fine now. However I do really agree with you. I don't know who moron written the SP that way. I'm at work debuggin the SP for one of my client therefore can not easily change the SP for an application working for years. However I do really agree with you. Thanks Kristen. I will definitely forward what you have written to the client.... |
 |
|
Kristen
Test
22859 Posts |
Posted - 2007-06-27 : 10:54:19
|
"Found the culprit statement"Damn and Blast! I saw that in the original this morning and forgot to mention it as being "Very inefficient and format-specific" Kristen |
 |
|
roswell
Starting Member
31 Posts |
Posted - 2007-06-27 : 12:06:17
|
I believe you could have mentioned it However JobDate between @StartDate and @EndDate seems to be working and sometimes not working :s I removed this line :S but ttell me the alternative for this, need to have searching based on start date and end date, I tried JobDate >=StartDate and JobDAte<=EndDate but its not working as well |
 |
|
Kristen
Test
22859 Posts |
Posted - 2007-06-27 : 14:01:13
|
"seems to be working and sometimes not working"How do you mean "not working"? Not selecting the right data perhaps??"Set @StartDate = Convert(Char(10),@StartDate,101)Set @EndDate = Convert(Char(10),@EndDate,101)"is format specific. Probably OK for your server, but not Locale-independent.The, rather long-winded but tested as being best-performing, way of removing the TIME for a DATE is:SELECT @StartDate = DATEADD(Day, 0, DATEDIFF(Day, 0, @StartDate))and for a "end date" you need to round UP to midnight, so:SELECT @EndDate = DATEADD(Day, 0, DATEDIFF(Day, 0, @EndDate)+1)and then you need to EXCLUDE any exactly-matching End Date ('coz it will be midnight and thus already "tomorrow), so:JobDate >= @StartDate and JobDate < @EndDatebut I dunno if that is related to your problem.Note that:If @EndDate = '1/1/1800'BeginSet @EndDate = DateAdd(Day,10,GetDate())Endis going to be 10 days hence, but at CURRENT time (OK, it then gets rounded, but I still think its not very bit "tight"!), and theIf @EndDate = '1/1/1800'stuff ought to be Locale-independent - i.e.If @EndDate = '18000101'or pedanticallyIf @EndDate = CONVERT(datetime, '18000101')but as with the "NA" stuff above I take a dim view of using a "real" date as a meaning for something else. Passing NULL to mean "Anything" would be better IMHOKristen |
 |
|
roswell
Starting Member
31 Posts |
Posted - 2007-06-28 : 06:22:52
|
How do you mean "not working"? Not selecting the right data perhaps??No, the SP again starting timeout and when I removed the whole statement i.e. JobDate between @StartDate and @EndDateIts working fine and not timeout so far (24 hrs passed)I'm following the rest of the thing you said.Thanks |
 |
|
|
|
|
|
|