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 |
|
jgonzalez14
Yak Posting Veteran
73 Posts |
Posted - 2011-08-25 : 11:07:12
|
Hello,I am trying to optimize the query below. This is taking way to long to execute. Any help would be appreciatedSELECT DISTINCT rs.DonorIDFROM dbo.RSIntegration_EligibleDonors rs WITH(NOLOCK) INNER JOIN dbo.RSIntegration_DonorListData rsdld (NOLOCK) ON rs.DonorID = rsdld.DonorID INNER JOIN dbo.Donor d WITH(NOLOCK) ON rs.DonorID = d.DonorID INNER JOIN dbo.DonorEligibilityDates deda WITH (NOLOCK) ON rs.DonorID = deda.DonorID INNER JOIN dbo.OpTypeDefinitions dedot WITH (NOLOCK) ON deda.EligibilityTypeID = dedot.EligibilityTypeID LEFT OUTER JOIN ( SELECT DISTINCT d.DonorID FROM dbo.Donor d WITH(NOLOCK) INNER JOIN dbo.DonorAssoc da WITH(NOLOCK) ON d.DonorID = da.DonorID INNER JOIN dbo.Donation do WITH(NOLOCK) ON da.DonorID = do.DonorID and da.HostID = do.HostID INNER JOIN dbo.Op o WITH(NOLOCK) ON da.HostID = o.HostID LEFT OUTER JOIN dbo.DonorDisAssoc dda WITH(NOLOCK) ON da.DonorID = dda.DonorID and da.HostID = dda.HostID WHERE ISNULL(da.HostID,0) <> 0 AND NOT do.ShowDateTime BETWEEN CONVERT(datetime,CONVERT(varchar(10),DATEADD(dd,-61,GETDATE()),(101))) AND CONVERT(datetime,CONVERT(varchar(10),GETDATE(),(101))) AND o.OpStartDateTime BETWEEN CONVERT(datetime,CONVERT(varchar(10),GETDATE(),(101))) and CONVERT(datetime,CONVERT(varchar(10),DATEADD(dd,60,GETDATE()),(101))) AND o.OpStatusID IN (1,3,8,9) AND dda.DonorID IS NULL ) ms ON rs.DonorID = ms.DonorIDWHERE d.Active=1 AND ISNULL(rs.Donor_Status,'') <> 'Deceased' AND (ISNULL(rs.LastVisitDate,'01/01/1900') >= DATEADD(dd,-730,CAST(CONVERT(varchar(50),GETDATE(),101) AS DATETIME))) AND ISNULL(d.isRecruitable,1) = 1 AND d.DOB <= CONVERT(datetime,CONVERT(varchar(10),DATEADD(YYYY,-18,GETDATE()),(101))) AND ISNULL(rs.HasPhone,'NO') = 'YES' AND NOT ISNULL(rs.IsOptedOutPhone,'NO') = 'YES' AND rs.Zip IN (SELECT ZipCode FROM dbo.fn_ZipsInRadius_Multi('32765,32701',10)) AND ( dedot.OpTypeID in (1,2,4) AND (ISNULL(rs.WBElig,'01/01/1900') <= DATEADD(dd,0,CAST(CONVERT(varchar(50),GETDATE(),101) AS DATETIME)))) AND ms.DonorID IS NULL AND NOT rs.DonorID IN (SELECT DISTINCT DonorID FROM dbo.DonorAttribute WHERE AttributeValueID = 112 ) AND NOT ISNULL(rsdld.NextAppointmentDate, '01/01/1900') >= CONVERT(datetime,CONVERT(varchar(10),GETDATE(),(101))) |
|
|
GilaMonster
Master Smack Fu Yak Hacker
4507 Posts |
Posted - 2011-08-25 : 11:19:34
|
| Table definitions and index definitions please.I assume, since you have nolock everywhere, that accuracy of data is not a major requirement here.--Gail ShawSQL Server MVP |
 |
|
|
Ifor
Aged Yak Warrior
700 Posts |
Posted - 2011-08-25 : 12:22:12
|
Using temp tables to hold intermediate results will probably work well here.Alternatively, try something like:SELECT d.DonorIDFROM dbo.Donor dWHERE d.Active = 1 AND d.DOB <= DATEADD(year, - 18, CURRENT_TIMESTAMP) AND (d.isRecruitable = 1 OR d.isRecruitable IS NULL) AND EXISTS ( SELECT 1 FROM dbo.RSIntegration_EligibleDonors rs WHERE d.DonorID = rs.DonorID AND rs.Donor_Status <> 'Deceased' AND rs.LastVisitDate >= DATEADD(day, DATEDIFF(d, 0, CURRENT_TIMESTAMP) -730, 0) AND rs.HasPhone = 'YES' AND rs.IsOptedOutPhone = 'YES' AND NOT rs.WBElig > DATEADD(day, DATEDIFF(d, 0, CURRENT_TIMESTAMP), 0) AND EXISTS ( SELECT 1 FROM dbo.fn_ZipsInRadius_Multi('32765,32701',10) M WHERE M.ZipCode = rs.Zip ) AND NOT EXISTS ( SELECT 1 FROM dbo.DonorAttribute Attrib WHERE Attrib.DonorID = rs.DonorID AND Attrib.AttributeValueID = 112 ) ) AND EXISTS ( SELECT 1 FROM dbo.RSIntegration_DonorListData rsdld WHERE r.DonorID = rsdld.DonorID AND NOT rsdld.NextAppointmentDate >= DATEADD(day, DATEDIFF(d, 0, CURRENT_TIMESTAMP), 0 ) AND EXISTS ( SELECT 1 FROM dbo.DonorEligibilityDates deda WHERE deda.DonorID = d.DonorID AND EXISTS ( SELECT 1 FROM dbo.OpTypeDefinitions dedot WHERE deda.EligibilityTypeID = dedot.EligibilityTypeID AND dedot.OpTypeID IN (1,2,4) ) ) AND NOT EXISTS ( SELECT 1 FROM dbo.DonorAssoc da WHERE da.DonorID = d.DonorID AND da.HostID <> 0 AND EXISTS ( SELECT 1 FROM dbo.Donation do WHERE da.DonorID = do.DonorID AND da.HostID = do.HostID AND NOT do.ShowDateTime BETWEEN DATEADD(day, DATEDIFF(d, 0, CURRENT_TIMESTAMP) -61, 0) AND DATEADD(day, DATEDIFF(d, 0, CURRENT_TIMESTAMP), 0) ) AND EXISTS ( SELECT 1 FROM dbo.Op o WHERE da.HostID = o.HostID AND o.OpStartDateTime BETWEEN DATEADD(day, DATEDIFF(d, 0, CURRENT_TIMESTAMP), 0) AND DATEADD(day, DATEDIFF(d, 0, CURRENT_TIMESTAMP) + 60, 0) AND o.OpStatusID IN (1,3,8,9) ) AND NOT EXISTS ( SELECT 1 FROM dbo.DonorDisAssoc dda WHERE da.DonorID = dda.DonorID AND da.HostID = dda.HostID ) ); |
 |
|
|
jgonzalez14
Yak Posting Veteran
73 Posts |
Posted - 2011-08-25 : 12:43:53
|
| I have narrowed it down to the left outer join ms table that I create from a subquery. When filtering ms.donorid is null this adds so much more processing time. Is there any other way to accmplish this? |
 |
|
|
Kristen
Test
22859 Posts |
Posted - 2011-08-25 : 13:43:21
|
Get rid of NOLOCK. It almost certainly isn't doing what you think it is, and what it is liable to do will give you sleepless nights forever.What you probably want, instead, is to set the database to READ_COMMITTED_SNAPSHOTThe DISTINCT is bad. ChangeSELECT DISTINCT rs.DonorIDtoSELECT COUNT(*), COUNT(DISTINCT rs.DonorID)Whats the COUNT(*)? That's the total number of records being retrieved, and then the DISTINCT is sorting them and throwing away the duplicates (to give you the number of records as per the COUNT(DISTINCT rs.DonorID) figure). Some of the criteria you run this for will probably be worse than others in this regard.There's another DISTINCT in there, in a sub query. Take that out too and see how that increases the COUNT(*)What IFor is showing you is the way to use EXISTS / NOT EXISTS instead of JOINSDon't use WHERE ISNULL(rs.Donor_Status,'') <> 'Deceased'If rs.Donor_Status is NULL then the test rs.Donor_Status <> 'Deceased' will fail. Same as when rs.Donor_Status has a value and is not "Deceased". So just users.Donor_Status <> 'Deceased'the extra benefit is that if rs.Donor_Status is indexed the Query Optimiser can use the index to speed up your query. If you useISNULL(rs.Donor_Status,'')then the index cannot be used. (That goes for wrapping any column in a function like that, in the WHERE clause or in the criteria for a JOIN)Putting the date ranges into an @Variable, and using the @Variable in the query, may be faster than having your forumla in the WHERE clause.ChangeCONVERT(datetime,CONVERT(varchar(10),DATEADD(dd,-61,GETDATE()),(101)))toDATEADD(Day, DATEDIFF(Day, 0, CURRENT_TIMESTAMP) -61, 0)(as IFor has done). Your method involves converting the date to a string and back again to a date. Its slow I'm afraid. The DATEADD / DATEDIFF method, whilst rather obtuse to read, just uses integer arithmetic, and is faster.If dbo.fn_ZipsInRadius_Multi() function contains trigonometry function (Since, Cosine, Tangent) then pre-calculate the co-ordinates for a square and use the top-left and bottom-right as a range in your WHERE clause. This will be much faster, and will be able to use an index for any X / Y co-ordinates stored in the table. Then, further refine the results using the more complex function which includes trigonometry - this will be operating on a much smaller data set, and the time for the Trigonometry will be reduced. Or use the Geospatial functions in SQL 2008AND NOT rs.DonorID IN (SELECT DISTINCT DonorID FROM dbo.DonorAttribute WHERE AttributeValueID = 112 )Using EXISTS will be faster than IN. Particularly as you have used DISTINCT. EXISTS can stop when it finds the first match. "SELECT DISTINCT DonorID" must find every possible value, sort them, remove duplicates, and then see if rs.DonorID is in the list (SQL is smarter than that, so it may not be that bad, but depending on the complexity of the WHERE clause SQL may not be able to take any shortcuts)But that's just some ideas to be considering. IFor's code has done all the work for you |
 |
|
|
denis_the_thief
Aged Yak Warrior
596 Posts |
Posted - 2011-08-25 : 14:23:42
|
quote: Originally posted by GilaMonsterI assume, since you have nolock everywhere, that accuracy of data is not a major requirement here.--Gail ShawSQL Server MVP
It does look scary given the context (i.e. Organ Donors) |
 |
|
|
GilaMonster
Master Smack Fu Yak Hacker
4507 Posts |
Posted - 2011-08-25 : 18:35:36
|
quote: Originally posted by denis_the_thief
quote: Originally posted by GilaMonsterI assume, since you have nolock everywhere, that accuracy of data is not a major requirement here.--Gail ShawSQL Server MVP
It does look scary given the context (i.e. Organ Donors)
Which do you think would be worse there, double-reading rows or missing rows?--Gail ShawSQL Server MVP |
 |
|
|
Transact Charlie
Master Smack Fu Yak Hacker
3451 Posts |
Posted - 2011-08-26 : 05:49:42
|
I tried to refactor this a little. I'm going to stop but I have rearranged the derived table quite a bit. I moved the LEFT JOIN and check for NULL into a NOT EXISTS Clause. This can speed things up many times if the columns involved are nullable. I removed a useless join to Donor table. And removed a WHERE ISNULL(x) check that was removed by the INNER JOIN condition anyway.SELECT rs.[DonorID]FROM dbo.RSIntegration_EligibleDonors AS rs JOIN dbo.RSIntegration_DonorListData AS rsdld ON rs.[DonorID] = rsdld.[DonorID] JOIN dbo.Donor AS d ON rs.[DonorID] = d.[DonorID] JOIN dbo.DonorEligibilityDates AS deda ON rs.[DonorID] = deda.[DonorID] JOIN dbo.OpTypeDefinitions AS dedot ON deda.[EligibilityTypeID] = dedot.[EligibilityTypeID] LEFT JOIN ( SELECT da.DonorID -- Removing useless reference to dbo.Donor in derived table FROM dbo.DonorAssoc da JOIN dbo.Op AS o ON da.HostID = o.HostID JOIN dbo.Donation do ON da.DonorID = do.DonorID AND da.HostID = do.HostID WHERE da.[HostID] <> 0 -- ISNULL(da.HostID,0) <> 0 This can't happen because of the inner join on da.[HostID] -> o.[HostID] This is probably useless -- GAH. Change those columns to DATETIME types. Then get rid of this CONVERT nonsense AND NOT do.ShowDateTime BETWEEN CONVERT(datetime,CONVERT(varchar(10),DATEADD(dd,-61,GETDATE()),(101))) AND CONVERT(datetime,CONVERT(varchar(10),GETDATE(),(101))) AND o.OpStartDateTime BETWEEN CONVERT(datetime,CONVERT(varchar(10),GETDATE(),(101))) and CONVERT(datetime,CONVERT(varchar(10),DATEADD(dd,60,GETDATE()),(101))) AND o.OpStatusID IN (1,3,8,9) -- AND dda.DonorID IS NULL (Moved Left JOIN to a NOT EXISTS clause, This has a big impact when the column(s) invoved can be null) AND NOT EXISTS ( SELECT 1 FROM dbo.DonorDisAddoc AS dda WHERE dda.[DonorID] = da.[DonorID] AND dda.[HostID] = da.[HostID] ) ) AS ms ON rs.DonorID = ms.DonorIDWHERE d.Active=1 AND ISNULL(rs.Donor_Status,'') <> 'Deceased' AND (ISNULL(rs.LastVisitDate,'01/01/1900') >= DATEADD(dd,-730,CAST(CONVERT(varchar(50),GETDATE(),101) AS DATETIME))) AND ISNULL(d.isRecruitable,1) = 1 AND d.DOB <= CONVERT(datetime,CONVERT(varchar(10),DATEADD(YYYY,-18,GETDATE()),(101))) AND ISNULL(rs.HasPhone,'NO') = 'YES' AND NOT ISNULL(rs.IsOptedOutPhone,'NO') = 'YES' AND rs.Zip IN (SELECT ZipCode FROM dbo.fn_ZipsInRadius_Multi('32765,32701',10)) AND ( dedot.OpTypeID in (1,2,4) AND (ISNULL(rs.WBElig,'01/01/1900') <= DATEADD(dd,0,CAST(CONVERT(varchar(50),GETDATE(),101) AS DATETIME)))) AND ms.DonorID IS NULL AND NOT rs.DonorID IN (SELECT DISTINCT DonorID FROM dbo.DonorAttribute WHERE AttributeValueID = 112 ) AND NOT ISNULL(rsdld.NextAppointmentDate, '01/01/1900') >= CONVERT(datetime,CONVERT(varchar(10),GETDATE(),(101)))Points:1) NULLABLE columns where I totally wouldn't expect them.... WHY? Your various [ID] columns can be NULL. REALLY? REALLY-REALLY? They aren't keys?2) Datatypes. STORE DATES as DATES / DATETIMES. Don't store them as VARCHARS, CHARS or any other nonsense.3) your WHERE clause is full of predicates wrapped in functions. You can't use any INDEXES because of this. See (1) and (2) for how to remedy this.4) Table definitions / Indexes. Do your tables even have primary keys and foreign keys? There are so many ISNULL wrappers here that I doubt it.Charlie===============================================================Msg 3903, Level 16, State 1, Line 1736The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION |
 |
|
|
denis_the_thief
Aged Yak Warrior
596 Posts |
Posted - 2011-08-26 : 08:57:18
|
quote: Originally posted by GilaMonsterWhich do you think would be worse there, double-reading rows or missing rows?--Gail ShawSQL Server MVP
Maybe displaying Data to the hospital staff that was rolled back instead of comitted. |
 |
|
|
|
|
|
|
|