Query Performance Issue with multiple unions and a view
-
Hello Gurus,
I have a case where instead of looking too much into the indexes(Tables are not that big), I am looking at the code itself because I believe that in the procedure and the view being called inside it has few unions referring to some common tables multiple times and I am trying to reduce the number of calls to it. Attached is the procedure code, the view code.
Can someone help me re-write the same so that the tables are referred only once or fewer times than at present. I looked into similar examples on this forum and found that a lot of unions with similar clause can be re-written using OR clause, I am looking for a push to think on those terms as I am a DBA and while a lot of people around me keep asking for indexes, I am trying to focus more on the code itself to begin with.
Regards
Chandan Jha
-
I would redesign this slightly, but a few things that jumps out at me are:
your query is hitting a view, which i try to avoid when possible. i prefer to include jsut the tables i need, just the columns i need, inside the proc itself.
your queries have OR statements in them, i try to avoid that:
AND (r.expirationDate IS NULL OR r.expirationDate >= @date)
you have one procedure to rule them all.
I think best practice is to have one procedure do one thing quickly.
Having one proc doing logical tests and doing different things, not so good for me.
i think you can get poor execution plans, based on the IF statements being different the next time it's called with other params.
i would create a master procedure, that calls three separate stored procs, based on your IF criteria.
i broke it out here as an example.
--#################################################################################################
-- capability_GetCapabilityByObject_prc master procedure governing three possible proc calls
--#################################################################################################
CREATE PROCEDURE [dbo].[capability_GetCapabilityByObject_prc] @userid INT,
@usergroupid INT,
@usercompanyid INT,
@includeExpired BIT=0
AS
BEGIN
SET NOCOUNT ON;
DECLARE @date AS DATETIME
SET @date = Getdate()
IF @includeExpired = 1
BEGIN
SET @date = Dateadd(year, -10, @date)
END
IF @usergroupid IS NULL
BEGIN
SET @usergroupid=0
END
IF @userid > 0
BEGIN
EXEC capability_GetCapabilityByObject_prc_ByUserID
@usergroupid,
@usercompanyid,
@includeExpired =0
END --IF @userid
ELSE IF usergroupid > 0
BEGIN
EXEC capability_GetCapabilityByObject_prc_ByUserGroupID
@usergroupid,
@usercompanyid,
@includeExpired =0
END --IF usergroupid
ELSE IF @usercompanyid > 0
BEGIN
EXEC capability_GetCapabilityByObject_prc_ByUserCompanyID
@usergroupid,
@usercompanyid,
@includeExpired =0
END --IF @usercompanyid
END --PROC
GO
--#################################################################################################
-- capability_GetCapabilityByObject_prc_ByUserID
--#################################################################################################
CREATE PROCEDURE capability_GetCapabilityByObject_prc_ByUserID @userid INT,
@usergroupid INT,
@usercompanyid INT,
@includeExpired BIT=0,
@date DATETIME
AS
BEGIN
SET NOCOUNT ON
SELECT r.capabilityId,
r.expirationDate,
r.effectiveDate,
c.capabilityName,
c.displayName,
'User' AS objectName,
1 AS [type],
ug.userGroupName AS [resource],
cs.capabilityStatusName,
r.CapabilityPackageId,
r.CapabilityPackageName
FROM dbo.RequestorToCapabilityDeferred_vw r (nolock)
INNER JOIN dbo.Users_tbl u (nolock)
ON r.requestorObjectId = u.personId
AND u.userCompanyId = @usercompanyid
INNER JOIN dbo.Capability_tbl c (nolock)
ON r.capabilityId = c.capabilityId
LEFT OUTER JOIN dbo.UserGroup_tbl ug (nolock)
ON r.resourceObjectId = ug.userGroupId
AND ug.userCompanyId = @usercompanyid
LEFT OUTER JOIN dbo.CapabilityStatus_tbl cs (nolock)
ON r.capabilityStatusid = cs.capabilityStatusid
WHERE u.userId = @userid
AND ( r.expirationDate IS NULL
OR r.expirationDate >= @date )
UNION ALL
SELECT r.capabilityId,
r.expirationDate,
r.effectiveDate,
c.capabilityName,
c.displayName,
utug.userGroupName AS objectname,
2 AS [type],
NULL AS [resource],
cs.capabilityStatusName,
r.CapabilityPackageId,
r.CapabilityPackageName
FROM dbo.UserToUserGroup_vw utug (nolock)
INNER JOIN dbo.Users_tbl u (nolock)
ON utug.userId = u.userId
AND u.userCompanyId = @usercompanyid
INNER JOIN dbo.RequestorToCapabilityDeferred_vw r (nolock)
ON utug.userGroupId = r.requestorObjectId
INNER JOIN dbo.Capability_tbl c(nolock)
ON r.capabilityId = c.capabilityId
LEFT OUTER JOIN dbo.CapabilityStatus_tbl cs(nolock)
ON r.capabilityStatusid = cs.capabilityStatusid
WHERE u.userId = @userid
AND ( r.expirationDate IS NULL
OR r.expirationDate >= @date )
UNION ALL
SELECT r.capabilityId,
r.expirationDate,
r.effectiveDate,
c.capabilityName,
c.displayName,
'Company' AS objectName,
3 AS [type],
NULL AS [resource],
CapabilityStatus_tbl.capabilityStatusName,
r.CapabilityPackageId,
r.CapabilityPackageName
FROM dbo.UserCompany_tbl uc(nolock)
INNER JOIN dbo.Users_tbl u(nolock)
ON uc.userCompanyId = u.userCompanyId
INNER JOIN dbo.RequestorToCapabilityDeferred_vw r (nolock)
ON uc.companyId = r.requestorObjectId
INNER JOIN dbo.Capability_tbl c (nolock)
ON r.capabilityId = c.capabilityId
LEFT OUTER JOIN dbo.CapabilityStatus_tbl (nolock)
ON r.capabilityStatusid = CapabilityStatus_tbl.capabilityStatusid
WHERE u.userId = @userid
AND ( r.expirationDate IS NULL
OR r.expirationDate >= @date )
ORDER BY [type],
objectname,
displayname
END
GO
--#################################################################################################
-- capability_GetCapabilityByObject_prc_ByUserGroupID
--#################################################################################################
CREATE PROCEDURE [dbo].[capability_GetCapabilityByObject_prc_ByUserGroupID] @userid INT,
@usergroupid INT,
@usercompanyid INT,
@includeExpired BIT=0,
@date DATETIME
AS
BEGIN
SET NOCOUNT ON;
SELECT r.capabilityId,
r.expirationDate,
r.effectiveDate,
c.capabilityName,
c.displayName,
'Company' AS objectName,
3 AS [type],
cs.capabilityStatusName,
r.CapabilityPackageId,
r.CapabilityPackageName
FROM dbo.Capability_tbl c (nolock)
INNER JOIN dbo.RequestorToCapabilityDeferred_vw r (nolock)
ON c.capabilityId = r.capabilityId
INNER JOIN dbo.UserCompany_tbl uc (nolock)
ON r.requestorObjectId = uc.companyId
INNER JOIN dbo.UserGroup_tbl ug (nolock)
ON ug.userCompanyId = uc.userCompanyId
LEFT OUTER JOIN dbo.CapabilityStatus_tbl cs (nolock)
ON r.capabilityStatusid = cs.capabilityStatusid
WHERE ( r.expirationDate IS NULL
OR r.expirationDate > @date )
AND ug.userGroupId = @usergroupid
UNION ALL
SELECT r.capabilityId,
r.expirationDate,
r.effectiveDate,
c.capabilityName,
c.displayName,
'Group' AS objectName,
2 AS [type],
cs.capabilityStatusName,
r.CapabilityPackageId,
r.CapabilityPackageName
FROM dbo.Capability_tbl c (nolock)
INNER JOIN dbo.RequestorToCapabilityDeferred_vw r (nolock)
ON c.capabilityId = r.capabilityId
LEFT OUTER JOIN dbo.CapabilityStatus_tbl cs(nolock)
ON r.capabilityStatusid = cs.capabilityStatusid
WHERE ( r.expirationDate IS NULL
OR r.expirationDate > @date )
AND r.requestorObjectId = @usergroupid
ORDER BY [type],
displayname
END
GO
--#################################################################################################
-- capability_GetCapabilityByObject_prc_ByUserCompanyID
--#################################################################################################
CREATE PROCEDURE [dbo].[capability_GetCapabilityByObject_prc_ByUserCompanyID] @userid INT,
@usergroupid INT,
@usercompanyid INT,
@includeExpired BIT=0,
@date DATETIME
AS
BEGIN
SET NOCOUNT ON;
SELECT r.capabilityId,
r.expirationDate,
r.effectiveDate,
c.capabilityName,
c.displayName,
'Company' AS objectName,
3 AS [type],
cs.capabilityStatusName,
r.CapabilityPackageId,
r.CapabilityPackageName
FROM dbo.Capability_tbl c (nolock)
INNER JOIN dbo.RequestorToCapabilityDeferred_vw r(nolock)
ON c.capabilityId = r.capabilityId
INNER JOIN dbo.UserCompany_tbl uc(nolock)
ON r.requestorObjectId = uc.companyId
LEFT OUTER JOIN dbo.CapabilityStatus_tbl cs(nolock)
ON r.capabilityStatusid = cs.capabilityStatusid
WHERE ( r.expirationDate IS NULL
OR r.expirationDate > @date )
AND uc.userCompanyId = @usercompanyid
ORDER BY displayname
END
GO
Lowell
--help us help you! If you post a question, make sure you include a CREATE TABLE... statement and INSERT INTO... statement into that table to give the volunteers here representative data. with your description of the problem, we can provide a tested, verifiable solution to your question! asking the question the right way gets you a tested answer the fastest way possible! -
Thanks Lowell. The NULL and the Or clause are business rules so won't be able to find my way around that.
I will try breaking the master proc and test with small procs, may be then the execution goes smooth. I thought that it might be possible to get rid of multiple calls to some common tables, but looks like this is not an option here. Let me try your suggestion and get back.
Regards
Chandan
-
As a side note you seem to be littering your database with NOLOCK hints. This is generally not a good idea, especially in things like views. That hint is far more than simply dirty reads. It can and will return missing and/or duplicate rows in addition to even more sinister problems. That hints has its place but more often than not it used when it is not fully understood. http://blogs.sqlsentry.com/aaronbertrand/bad-habits-nolock-everywhere/[/url]
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/ -
chandan_jha18 (3/21/2016)
Thanks Lowell. The NULL and the Or clause are business rules so won't be able to find my way around that.I will try breaking the master proc and test with small procs, may be then the execution goes smooth. I thought that it might be possible to get rid of multiple calls to some common tables, but looks like this is not an option here. Let me try your suggestion and get back.
Regards
Chandan
Lowell's suggestion is likely to have a significant effect on the performance of your stored procedure. As for eliminating the UNIONs in your view, I don't think that's possible because the lead table for each query is different. If you're still experiencing performance problems after splitting up the sproc, post up the actual execution plans as .sqlplan files.
“Write the query the simplest way. If through testing it becomes clear that the performance is inadequate, consider alternative query forms.” - Gail ShawFor fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden -
ChrisM@Work (3/21/2016)
chandan_jha18 (3/21/2016)
Thanks Lowell. The NULL and the Or clause are business rules so won't be able to find my way around that.I will try breaking the master proc and test with small procs, may be then the execution goes smooth. I thought that it might be possible to get rid of multiple calls to some common tables, but looks like this is not an option here. Let me try your suggestion and get back.
Regards
Chandan
Lowell's suggestion is likely to have a significant effect on the performance of your stored procedure. As for eliminating the UNIONs in your view, I don't think that's possible because the lead table for each query is different. If you're still experiencing performance problems after splitting up the sproc, post up the actual execution plans as .sqlplan files.
Thanks Chris. It will take me sometime to implement Lowell's plan and I will report on that tomorrow. Meanwhile, here is the execution plan attached for the original procedure.
-
The execution plan shows at least two Cartesian products probably because of insufficient criteria. You can see these areas in the execution plan quite easily... they're the ones with the big fat lines coming off a couple of MERGE Joins. Since they look similar, it might be in the view but haven't done a deep dive on it.
According to the single row lines on everything else, it looks like a whole lot of RBAR going on. Some of that may be valid but, if it isn't (haven't looked at the proc yet), you may have other problems, as well.
My recommendation is "Divide'n'Conquer" even if you have to store interim or "pre" result sets in Temp Tables because the proc is beating the hell out of TempDB with hash and merge joins that result in Cartesian products.
--Jeff Moden
RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.Change is inevitable... Change for the better is not.
Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally) -
As per Lowell's suggestion, I tried to create stored procedures but on lower env. the build team started the refresh so I am waiting to finish them in 2-3 hours.
However, taking notes from diving this piece into chunks, I tried to create a table and dump all the data from the view to it and the procedure is running amazingly fast. And I am not seeing any hash or merge joins fat pipes.
Also, I am going to ask the business if 1 hour or 30 minutes stale data is acceptable. If they respond positively, I can schedule a job to populate a real table with view data and then make a switch from existing to new one. As on today, this step takes me 10 seconds.
But I am still going to try what Lowell suggested, 3 small procs and see how it behaves. I have had bad experience with views used in complex queries and this one is proving to be no different.
Thanks
Chandan
-
Lowell\Chris- As per Lowell's advice I created smaller procedures and attaching the new plan. however, as Jeff pointed out those hash and merge join fat pipes appearing again. Not sure how Jeff said it is RBRAR , but as soon as I convert the view to a table with data and an index, the original procedure just runs like a rocket.
Attached is the plan.
Thanks
Chandan
-
chandan_jha18 (3/22/2016)
Lowell\Chris- As per Lowell's advice I created smaller procedures and attaching the new plan. however, as Jeff pointed out those hash and merge join fat pipes appearing again. Not sure how Jeff said it is RBRAR , but as soon as I convert the view to a table with data and an index, the original procedure just runs like a rocket.Attached is the plan.
Thanks
Chandan
This new query exhibits an optimiser timeout also.
Funny thing is, it's quite different to everything else you've posted from what I remember. There's sufficient common ground between the three UNIONed SELECTs to split out a temp table, to make it much simpler and probably faster too. Since it's now much simpler, you may find it possible to rework it as a single query. That would be my goal.
SELECTr.capabilityId,
r.expirationDate,
r.effectiveDate,
c.capabilityName,
c.displayName,
--'User' AS objectName,
--1 AS [type],
--ug.userGroupName AS [resource],
cs.capabilityStatusName,
r.CapabilityPackageId,
r.CapabilityPackageName,
r.requestorObjectId,
r.resourceObjectId
INTO #Temp
FROM dbo.RequestorToCapabilityDeferred_vw r
INNER JOIN dbo.Capability_tbl c
ON r.capabilityId = c.capabilityId
LEFT OUTER JOIN dbo.CapabilityStatus_tbl cs
ON r.capabilityStatusid = cs.capabilityStatusid
WHERE (r.expirationDate IS NULL OR r.expirationDate >= @date)
--------------------------------------------------------------------------
SELECT
r.capabilityId,
r.expirationDate,
r.effectiveDate,
r.capabilityName,
r.displayName,
'User' AS objectName,
1 AS [type],
ug.userGroupName AS [resource],
r.capabilityStatusName,
r.CapabilityPackageId,
r.CapabilityPackageName
FROM #Temp r
INNER JOIN dbo.Users_tbl u
ON r.requestorObjectId = u.personId
LEFT OUTER JOIN dbo.UserGroup_tbl ug
ON r.resourceObjectId = ug.userGroupId
WHERE u.userId = @userid
AND u.userCompanyId = @usercompanyid
AND ug.userCompanyId = @usercompanyid
UNION ALL
SELECT
r.capabilityId,
r.expirationDate,
r.effectiveDate,
r.capabilityName,
r.displayName,
utug.userGroupName AS objectname,
2 AS [type],
NULL AS [resource],
r.capabilityStatusName,
r.CapabilityPackageId,
r.CapabilityPackageName
FROM #Temp r
INNER JOIN dbo.UserToUserGroup_vw utug
ON utug.userGroupId = r.requestorObjectId
INNER JOIN dbo.Users_tbl u
ON utug.userId = u.userId
WHERE u.userId = @userid
AND u.userCompanyId = @usercompanyid
UNION ALL
SELECT
r.capabilityId,
r.expirationDate,
r.effectiveDate,
r.capabilityName,
r.displayName,
'Company' AS objectName,
3 AS [type],
NULL AS [resource],
r.capabilityStatusName,
r.CapabilityPackageId,
r.CapabilityPackageName
FROM #Temp r
INNER JOIN dbo.UserCompany_tbl uc (nolock)
ON uc.companyId = r.requestorObjectId
INNER JOIN dbo.Users_tbl u (nolock)
ON uc.userCompanyId = u.userCompanyId
WHERE u.userId = @userid
ORDER BY [type],
objectname,
displayname
“Write the query the simplest way. If through testing it becomes clear that the performance is inadequate, consider alternative query forms.” - Gail ShawFor fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden -
--posted to wrong thread--
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/ -
Every query has a common block:
FROM dbo.RequestorToCapabilityDeferred_vw rINNER JOIN dbo.Capability_tbl c ON r.capabilityId = c.capabilityId
LEFT OUTER JOIN dbo.CapabilityStatus_tbl CS on r.capabilityStatusid = CS.capabilityStatusid
It should be made into a view, or at least a derived table.
View definition should contain definitions of dbo.RequestorToCapabilityDeferred_vw and be used instead if it in this procedure.
And the performance problems seem to come from that view.
Without seeing its code there is a little chance to figure out a fix.
_____________
Code for TallyGenerator -
Sergiy (3/23/2016)
Every query has a common block:FROM dbo.RequestorToCapabilityDeferred_vw rINNER JOIN dbo.Capability_tbl c ON r.capabilityId = c.capabilityId
LEFT OUTER JOIN dbo.CapabilityStatus_tbl CS on r.capabilityStatusid = CS.capabilityStatusid
It should be made into a view, or at least a derived table.
View definition should contain definitions of dbo.RequestorToCapabilityDeferred_vw and be used instead if it in this procedure.
And the performance problems seem to come from that view.
Without seeing its code there is a little chance to figure out a fix.
Thanks for replying. I have put the code and execution plan on page-1 of this thread.
-
Dear All,
Today while running the procedure in its original form, which takes around 10 seconds to output 100 rows for a particular set of parameters, I opened a new query window and ran the following:
select * from sys.dm_os_waiting_tasks where session_id=my spid.
This was giving waits on 'CXPACKETS' . I tried to interpret the resource_description field which had similar values:
exchangeEvent id=Pipec9dabf000 WaitType=e_waitPipeGetRow nodeId=134
exchangeEvent id=Pipec9dabf000 WaitType=e_waitPipeGetRow nodeId=134
Out of a bad habit of using maxdop hint when seeing such wait types, I put a hint in the procedure with MAXDOP(1) and the query is able to return results within 1 second.
So I have two possible solutions at this point:
- Use a permanent cache table to populate the data from view every 15 or 30 minutes(takes 5-10 seconds) and refer that in place of view in the procedure code
- Use maxdop hint.
Sorry for the long break, we had a major festival last week:-)
Regards
Chandan
-
chandan_jha18 (3/21/2016)
Thanks Lowell. The NULL and the Or clause are business rules so won't be able to find my way around that.Went back through the posts on this thread and wanted to make another point...
Understood on the business rules requirement but, be advised, that's frequently a performance killer and you and the business rule people are going to need to fix it. One fairly painless way to do such a thing is to make the column NOT NULL and change all "unknown" dates to a future date like '9999' (converts to 9999-01-01, which leaves a bit of headroom for other datetime tricks, is easy to type, and is easy to remember).
If you can't even do that, then you should quickly isolate the rows that are NULL in one query and the rows that are greater than today in another, possibly s a UNION ALL and possibly dumped into a Temp Table if especially if referred to more than once elsewhere in the proc.
--Jeff Moden
RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.Change is inevitable... Change for the better is not.
Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)
Viewing 15 posts - 1 through 15 (of 16 total)
You must be logged in to reply to this topic. Login to reply