Stored Proc - Performance Check up

This stored proc was not written by me but it always shows up in my alerts where there are many query plans created for it.

Any suggestions on how to make this better.  Im pretty sure these joins and where clauses are bad..but not sure how to get around.


USE [cadoc_system]
GO
/****** Object:  StoredProcedure [dbo].[spGetMRUDocList3]    Script Date: 11/7/2017 9:43:22 AM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER procedure [dbo].[spGetMRUDocList3]

		@nSubScr int
	, @cRootSite varchar(50)
	, @nMaxRecords int = 10

	
as
	set nocount on 
			
			(select distinct e.cFileAreaSiteCode, e.ctitle, e.cdocumentid,
		   c.nid, c.cName, c.cFEIN,
			 e.dCreated, d.taxyear, d.typeid, 
			   isnull(r.nRating,0) ranking, d.ndocument
			from tevents e
			join tSubscr s on e.nSubScr = s.nid
			join tSiteXCrmClient x on e.cFileAreaSiteCode = cast(x.nidSite as varchar(6))
			join cadoc_crm..tclient c on c.nid = x.nIdClient
			join table...documents d on e.cDocumentID = d.documentid

			left outer join tSubscrXDocumentRating r on r.nIdDocument = e.nid and  r.nIdSubscr = @nSubScr 
			
			
			where s.nid = @nSubScr
			and e.dCreated >= s.MRUClearDateDocuments
			and e.cdocumentid like 'DOC%'
			and e.ctitle != ''
			and e.dCreated > s.MRUClearDateDocuments 
			and d.nlinkeddoc = 0
			and d.ndocstatus != 99
			and d.sitecode =  @cRootSite
			and e.dCreated in 

			(select max(ee.dCreated) from tevents ee
			join tSubscr ss on ee.nSubScr = ss.nid
			where ss.nid = @nSubScr
			and ee.cSiteCode = @cRootSite
			group by ee.cDocumentID )
			and c.cSiteCode = @cRootSite
			and e.cFileAreaSiteCode not like 'CAT%'
			) 

		ORDER BY cTitle

Open in new window

LVL 11
Robb HillSenior .Net DeveloperAsked:
Who is Participating?

[Product update] Infrastructure Analysis Tool is now available with Business Accounts.Learn More

x
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

Jim HornMicrosoft SQL Server Developer, Architect, and AuthorCommented:
Define 'better' and 'bad', as posting a pile of T-SQL and asking us to 'improve this' without any other guidance is not an abundantly actionable question.
0
Vitor MontalvãoMSSQL Senior EngineerCommented:
Do you have the related Query Execution Plan?
I can also see a CAST function on a JOIN clause, meaning that any index on nidSite field wouldn't be used because of the CAST:
join tSiteXCrmClient x on e.cFileAreaSiteCode = cast(x.nidSite as varchar(6))

Open in new window

1
Pawan KumarDatabase ExpertCommented:
three things

DISTINCT - use group by
Casting in the join condition - we should have used proper data types.. or an extra column
Order by  - ORDER BY cTitle - Is it required ? Do you have an index on this columns?

Also please provide actual execution plan for this.
1
Big Business Goals? Which KPIs Will Help You

The most successful MSPs rely on metrics – known as key performance indicators (KPIs) – for making informed decisions that help their businesses thrive, rather than just survive. This eBook provides an overview of the most important KPIs used by top MSPs.

Scott PletcherSenior DBACommented:
First let's try moving the WHERE conditions for JOINed tables into the JOINs, trying to reduce the number of matching rows as quickly as possible.  I've also explicitly specified:
e.nSubScr = @nSubScr
which can be derived from the original conditions, and thus SQL is probably (hopefully) already doing this, but it won't hurt to do it ourselves.

Any other changes would require additional knowledge about the table structures and indexes.  Btw, an index on cTitle would not avoid a sort here, so don't bother creating one.
2
Robb HillSenior .Net DeveloperAuthor Commented:
Here is my refactoring so far:

select  e.cFileAreaSiteCode, e.ctitle, e.cdocumentid,e.dCreated,e.nSubScr, s.nid,e.cAction,
              c.nid, c.cName, c.cFEIN,
              d.taxyear, d.typeid, d.ndocument, isnull(r.nRating,0) ranking
                     from cadoc_system..tevents e 
                     inner join cadoc_system..tSubscr t
                           on e.nSubScr = t.nid
                     inner join cadoc_system..tsite s
                           on  e.cFileAreaSiteCode = s.cSiteCode
                     inner join cadoc_system..tSiteXCrmClient xc
                           on  xc.nidSite = s.nid and s.cPSiteCode = xc.cSiteCode
                     inner join cadoc_crm..tClient c
                           on xc.nIdClient = c.nid
                     inner join table..documents d 
                           on e.cDocumentID = d.documentid
                     left outer join cadoc_system..tSubscrXDocumentRating r
                           on r.nIdDocument = e.nid and  r.nIdSubscr = 112971

                     where t.nid = 112971
                     and e.dCreated >= t.MRUClearDateDocuments
                     and e.cdocumentid like 'DOC%'
                     and e.ctitle != ''
                     and e.dCreated > t.MRUClearDateDocuments 
                     and d.ndocstatus != 99
                     and d.sitecode =  '21686'
                     and d.nlinkeddoc = 0

                     and e.dCreated in 

                     (select max(ee.dCreated) from cadoc_system..tEvents ee
                     join cadoc_system..tSubscr ss on ee.nSubScr = ss.nid
                     where ss.nid = 112971
                     and ee.cSiteCode = '21686'
                     group by ee.cDocumentID )
                     and c.cSiteCode = '21686'
                     and e.cFileAreaSiteCode not like 'CAT%'

Open in new window

0
Robb HillSenior .Net DeveloperAuthor Commented:
Im not sure how to move these where statements to joins
0
Vitor MontalvãoMSSQL Senior EngineerCommented:
Im not sure how to move these where statements to joins
I wouldn't care about that.

Did you see any improvement with the new query?
Also, don't forget the Query Execution Plan.
0
Scott PletcherSenior DBACommented:
DOH, i had the statement rewritten as I described it and then failed to post it.  Grrrr.

Here's the original with a quick re-write.  I haven't looked at the last query you posted yet.

I strongly suspect this subquery is causing performance issues, but not sure how to adjust it yet:
                  (select max(ee.dCreated) from tevents ee
                  join tSubscr ss on ee.nSubScr = ss.nid
                  where ss.nid = @nSubScr
                  and ee.cSiteCode = @cRootSite
                  group by ee.cDocumentID)


			
           (select distinct e.cFileAreaSiteCode, e.ctitle, e.cdocumentid,
		   c.nid, c.cName, c.cFEIN,
			 e.dCreated, d.taxyear, d.typeid, 
			   isnull(r.nRating,0) ranking, d.ndocument
			from tevents e
			join tSubscr s on e.nSubScr = s.nid and s.nid = @nSubScr and e.dCreated > s.MRUClearDateDocuments
			join tSiteXCrmClient x on e.cFileAreaSiteCode = cast(x.nidSite as varchar(6))
			join cadoc_crm..tclient c on c.nid = x.nIdClient and c.cSiteCode = @cRootSite
			join table...documents d on e.cDocumentID = d.documentid and d.nlinkeddoc = 0 and d.ndocstatus != 99 and d.sitecode = @cRootSite

			left outer join tSubscrXDocumentRating r on r.nIdDocument = e.nid and  r.nIdSubscr = @nSubScr 			
			
			where e.NSubScr = @nSubScr
			and e.cdocumentid like 'DOC%'
			and e.ctitle != ''
                    and e.cFileAreaSiteCode not like 'CAT%'
			and e.dCreated in 
			(select max(ee.dCreated) from tevents ee
			join tSubscr ss on ee.nSubScr = ss.nid
			where ss.nid = @nSubScr
			and ee.cSiteCode = @cRootSite
			group by ee.cDocumentID )
			) 
		ORDER BY cTitle

Open in new window

1

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
Scott PletcherSenior DBACommented:
Just took a look at your new code, it looks right, with the table row-limiting conditions in the table join whenever possible, except for the "first"/anchor table.


@Vitor:
Im not sure how to move these where statements to joins

I wouldn't care about that.

Really?  The idea is to reduce as quickly as possible the number of rows that have to be processed.  And, as I understood MS's explanation of SQL's logical sequence of operations, WHERE conditions are typically processed after the joins, meaning a lot more rows would need processed if the conditions aren't moved to the joins.  Typically the first/anchor table is an exception, because for obvious reasons of efficiency SQL applies conditions to that table as part of the initial read of rows.  For other tables, SQL may apply WHERE conditions first, but it's not guaranteed, and can occur only if SQL "knows" it won't cause different results, which sometimes SQL can't definitely determine.

This is the logical order of SQL operations that MS has stated for years, excluding recent additions such as ctes and clr, etc .:

1. FROM  
2. ON
3. OUTER
4. WHERE
5. GROUP BY
6. CUBE | ROLLUP
7. HAVING
8. SELECT
9. DISTINCT
10 ORDER BY
11. TOP
1
Vitor MontalvãoMSSQL Senior EngineerCommented:
@Scott, I don't see it like that. In fact I made some tests before to see what's the difference between filtering in JOIN clause and WHERE clause and didn't find any. The Query Execution Plan and time was always the same no matter if I filter in JOIN or WHERE clause.
0
Robb HillSenior .Net DeveloperAuthor Commented:
Revised thus far:

/* NEW STORE PROC */
select  e.cFileAreaSiteCode, e.ctitle, e.cdocumentid,e.dCreated,e.nSubScr, s.nid,e.cAction,
              c.nid, c.cName, c.cFEIN,
              d.taxyear, d.typeid, d.ndocument, isnull(r.nRating,0) ranking
                     from cadoc_system..tevents e 
                     inner join cadoc_system..tSubscr t
                           on e.nSubScr = t.nid and t.nid = 112971 and e.dCreated > t.MRUClearDateDocuments
                     inner join cadoc_system..tsite s
                           on  e.cFileAreaSiteCode = s.cSiteCode
                     inner join cadoc_system..tSiteXCrmClient xc
                           on  xc.nidSite = s.nid and s.cPSiteCode = xc.cSiteCode
                     inner join cadoc_crm..tClient c
                           on xc.nIdClient = c.nid and c.cSiteCode = '21686'
                     inner join table..documents d 
                           on e.cDocumentID = d.documentid and d.nlinkeddoc = 0 and d.ndocstatus != 99 and d.sitecode = '21686'
                     left outer join cadoc_system..tSubscrXDocumentRating r
                           on r.nIdDocument = e.nid and  r.nIdSubscr = 112971

                     where e.nSubScr = 112971                    
                     and e.cdocumentid like 'DOC%'
                     and e.ctitle != ''                             

                     and e.dCreated in 

                     (select max(ee.dCreated) from cadoc_system..tEvents ee
                     join cadoc_system..tSubscr ss on ee.nSubScr = ss.nid
                     where ss.nid = 112971
                     and ee.cSiteCode = '21686'
                     group by ee.cDocumentID )
                     and c.cSiteCode = '21686'
                     and e.cFileAreaSiteCode not like 'CAT%'

Open in new window

0
Scott PletcherSenior DBACommented:
I think these conditions from the WHERE:
                    and c.cSiteCode = '21686'
                     and e.cFileAreaSiteCode not like 'CAT%'
can go in join to the "c" table.

Have you tried it?  Did it perform any better?

I think any further improvements will require looking at table and index definitions.
0
Scott PletcherSenior DBACommented:
@vitor:
@Scott, I don't see it like that. In fact I made some tests before to see what's the difference between filtering in JOIN clause and WHERE clause and didn't find any. The Query Execution Plan and time was always the same no matter if I filter in JOIN or WHERE clause.

Frankly how you see it doesn't matter.  It's up to the SQL engine itself.  And the people that wrote the engine have stated how it works.  I believe what they say is true, whether you think it's true or not.

Now, MS also says, as I said above, that if SQL can determine that it can safely apply WHERE conditions sooner and get better performance from it, it will do that.  But it only does that if it can be absolutely sure that it won't give an incorrect result from that.  And that makes sense too: above all else, SQL never wants to give a false result.

Also, keep in mind that if you have views within views within views ... or some other really complex query structure, SQL sometimes "times out" on trying to optimize and just uses a basic plan.  Such a basic plan might not shift all WHERE clauses around for best performance.

Furthermore, for OUTER joins, you must put the conditions in the join or get wrong results.  For example, in the above query:
                     left outer join cadoc_system..tSubscrXDocumentRating r
                           on r.nIdDocument = e.nid and  r.nIdSubscr = 112971
If you move the check on r.NIdSubscr to the WHERE clause, you could change the results of the query (because you've effectively made it an INNER JOIN, because all WHERE conditions must be true for a row to be SELECTed). (Note that the original query correctly had it in the ON already, as required.)

Given all that, I'd strongly recommend that people put it in the JOIN clause to assure best performance all the time.  If you really want to, specify it in both places, although that makes it tougher to keep the code consistent.
1
Robb HillSenior .Net DeveloperAuthor Commented:
Thanks for all your comments and discussion on the issue.  Very informative as well as truly helped the performance of this old proc.
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Query Syntax

From novice to tech pro — start learning today.