Question :
I have a SQL Server 2017 Enterprise Edition instance where a stored procedure was taking approx. five minutes to execute. After reviewing the stored procedure code, I could see that there was an inline scalar UDF referenced several times in the SELECT list and also in the predicate WHERE clause of the stored procedure body.
I advised the application team owning the code that they should refactor their stored proc not to use an inline UDF which they took onboard and replaced with a TVF. Whilst they were doing this, I noticed that the application database still had database compatibility level 100 so I raised this to the latest level of 140 after running the database through the Data Migration Assistant to check for deprecated features and breaking changes.
Following the replacement of the UDF for a TVF and raising the database compatibility level from 100 to 140, the performance has increased greatly and the stored proc now executes in under a minute but performance is still not where I’d like to it be. I’m hoping someone might be able to advise of anything obvious I’m missing or point me in the right direction of anything else I can do to further optimise the code or get this to perform better? The execution plan is here: https://www.brentozar.com/pastetheplan/?id=ByrsEdRpr
The code for the stored proc and function are as below and the stored procedure is called by the application as such: “EXEC dbo.CAOT_GetApplicationQueue;1”
/****** Object: StoredProcedure [dbo].[CAOT_GetApplicationQueue] ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE PROCEDURE [dbo].[CAOT_GetApplicationQueue]
(@userID VARCHAR(50)='', @showComplete CHAR(1)='N', @JustMyQueue BIT=0, @ChannelId VARCHAR(10) = NULL )
AS
BEGIN
SELECT App.pkApplication ,
COALESCE(ApplicationReference, AlternateApplicationReference) AS ApplicationReference ,
ApplicationDate ,
Name ,
Telephone ,
[Address] ,
Email ,
CIN ,
Dob ,
CreatedDate ,
BusinessPhone ,
PostCode ,
MobilePhone ,
[Action] ,
ActionStatus ,
branchNumber ,
AccountNumber ,
AccountType ,
act.accountDescription,
IsNull( appstatus.DESCRIPTION ,'-- CREATED --') As LastStatus,
IsNull(appstatus.DAYS,'0') DaysSinceLastStatus ,
DATEDIFF(d,ApplicationDate, GETDATE()) DaysCreated,
InitialUserID,
IsNull(appstatus.STATUS,'-- MADE --') APPLICATIONSTATUS
FROM dbo.CAOT_Application (NOLOCK) app
LEFT OUTER JOIN dbo.CAOT_AccountType (NOLOCK) act
ON app.AccountType = act.AccountTypecode
LEFT OUTER JOIN [CAOT_GetAllApplicationStatus]() appstatus
ON app.pkApplication = appstatus.[PKAPPLICATION]
WHERE (IsNull(appstatus.STATUSCODE,'MADE') NOT IN ('CANCELLED','DECLINED','COMPLETE','EXPIRED')
OR @showComplete='Y') AND
(@JustMyQueue = 0 OR InitialUserID = @userID) AND
(@ChannelId IS NULL OR ChannelID = @ChannelId OR (@ChannelId = 'CBU' AND ChannelID IS NULL AND isCAO='N'))
ORDER BY CASE WHEN InitialUserID = @userid THEN 10 ELSE 900 END, ApplicationDate DESC
END
GO
/****** Object: UserDefinedFunction [dbo].[CAOT_GetAllApplicationStatus] ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE FUNCTION [dbo].[CAOT_GetAllApplicationStatus]() RETURNS
@return TABLE
(
[PKAPPLICATION] [int] NOT NULL,
[PKAPPLICATIONEVENT] INT,
[EVENTCREATEDDATE] [DATETIME] NULL,
[KEY] VARCHAR(12) NULL,
[DESCRIPTION] VARCHAR(200) NULL,
[CODE] VARCHAR(20) NULL,
[DAYS] VARCHAR(20) NULL,
[STATUS] VARCHAR(200) NULL,
[STATUSCODE] VARCHAR(50) NULL
)
AS
BEGIN
Declare @AppStatus table
(
[PKAPPLICATION] [int] NOT NULL,
[PKAPPLICATIONEVENT] INT,
[EVENTCREATEDDATE] [DATETIME] NULL,
[KEY] VARCHAR(12) NULL,
[DESCRIPTION] VARCHAR(200) NULL,
[CODE] VARCHAR(20) NULL,
[DAYS] VARCHAR(20) NULL,
[STATUS] VARCHAR(200) NULL,
[STATUSCODE] VARCHAR(50) NULL
)
INSERT INTO @AppStatus
SELECT
fkApplication,
ev.pkApplicationEvent As pkApplicationEvent,
ev.CreateDate As 'EventCreatedDate',
CONVERT(VARCHAR(12), evt.fkApplicationStatus) As 'KEY',
evt.EventDescription As 'DESCRIPTION',
evt.EventCode As 'CODE' ,
CONVERT(VARCHAR(20), DATEDIFF(d, ev.createdate, GETDATE()) ) As 'DAYS',
apps.StatusDescription As 'STATUS' ,
apps.StatusCode As 'STATUSCODE'
FROM dbo.CAOT_ApplicationEvent (NOLOCK) ev
INNER JOIN dbo.CAOT_EventType (NOLOCK) evt ON ev.fkEventType = evt.pkEventType
INNER JOIN dbo.CAOT_ApplicationStatus (NOLOCK) apps ON evt.fkApplicationStatus = apps.pkApplicationStatus
ORDER BY ev.CreateDate DESC, ev.pkApplicationEvent DESC
INSERT INTO @return
Select * from @AppStatus AllStatus
Where AllStatus.EVENTCREATEDDATE = ( Select Max(LatestAppStatus.EVENTCREATEDDATE) from @AppStatus LatestAppStatus where LatestAppStatus.PKAPPLICATION =AllStatus.PKAPPLICATION ) --Z On X.PKAPPLICATION = Z.PKAPPLICATION
RETURN
END
GO
Answer :
You can replace your TVF with a view (or keep the TVF, but use the view for your performance-critical sproc):
CREATE VIEW CAOT_AllApplicationStatuses AS
SELECT
fkApplication,
ev.pkApplicationEvent AS pkApplicationEvent,
ev.CreateDate AS EventCreatedDate,
CONVERT(VARCHAR(12), evt.fkApplicationStatus) As 'KEY',
evt.EventDescription AS 'DESCRIPTION',
evt.EventCode AS 'CODE',
CONVERT(VARCHAR(20), DATEDIFF(d, ev.createdate, GETDATE())) AS 'DAYS',
apps.StatusDescription AS 'STATUS',
apps.StatusCode AS 'STATUSCODE'
FROM
dbo.CAOT_ApplicationEvent (NOLOCK) ev
INNER JOIN dbo.CAOT_EventType (NOLOCK) evt ON ev.fkEventType = evt.pkEventType
INNER JOIN dbo.CAOT_ApplicationStatus (NOLOCK) apps ON evt.fkApplicationStatus = apps.pkApplicationStatus
WHERE
NOT EXISTS
(
SELECT * FROM dbo.CAOT_ApplicationEvent AS LaterEvent WHERE EV.pkApplication = LaterEvent.pkApplication AND LaterEvent.pkApplication.CreateDate > EV.CreateDate
)
ORDER BY
ev.CreateDate DESC, ev.pkApplicationEvent DESC
This is simply the content of the TVF’s main SELECT
query, with the WHERE
clause from the second SELECT
incorporated as a NOT EXISTS
. I’m trusting that all records in CAOT_ApplicationEvent
have records in CAOT_EventType
and CAOT_ApplicationStatus
; if that’s not the case, you’ll need to add those joins in the NOT EXISTS
query.
Just using a view rather than a TVF may help, as the parser will incorporate the view into the final query, and discard unused parts; those calls to CONVERT()
, for example, are likely to be relatively expensive, but they appear to be unused. However, the complex predicates in your top-level sproc may necessitate a table scan. Let’s give this a shot and see if it needs more work!
After reading http://sommarskog.se/dyn-search.html
you could split the query into multiple parts, something like:
create procedure [dbo].[CAOT_GetApplicationQueue] ( @userID varchar(50) = '', @showComplete char(1) = 'N', @JustMyQueue bit = 0, @ChannelId varchar(10) = null )
as begin
select case when InitialUserID = @userID then 10 else 900 end as SortCol, app.pkApplication, coalesce(ApplicationReference, AlternateApplicationReference) as ApplicationReference, ApplicationDate, Name, Telephone, [Address], Email, CIN, Dob, CreatedDate, BusinessPhone, PostCode, MobilePhone, [Action], ActionStatus, branchNumber, AccountNumber, AccountType
, act.accountDescription, isnull(appstatus.DESCRIPTION, '-- CREATED --') as LastStatus, isnull(appstatus.DAYS, '0') DaysSinceLastStatus, datediff(d, ApplicationDate, getdate()) DaysCreated, InitialUserID, isnull(appstatus.STATUS, '-- MADE --') APPLICATIONSTATUS
from dbo.CAOT_Application app
left outer join dbo.CAOT_AccountType act on app.AccountType = act.AccountTypecode
left outer join [CAOT_GetAllApplicationStatus]() appstatus on app.pkApplication = appstatus.[PKAPPLICATION]
where ( isnull(appstatus.STATUSCODE, 'MADE') not in ( 'CANCELLED', 'DECLINED', 'COMPLETE', 'EXPIRED' ) or @showComplete = 'Y' )
and ( @JustMyQueue = 0 /* or InitialUserID = @userID */)
and ( @ChannelId is null or ChannelID = @ChannelId or ( @ChannelId = 'CBU' and ChannelID is null and isCAO = 'N' ))
union all
select 10 as SortCol, app.pkApplication, coalesce(ApplicationReference, AlternateApplicationReference) as ApplicationReference, ApplicationDate, Name, Telephone, [Address], Email, CIN, Dob, CreatedDate, BusinessPhone, PostCode, MobilePhone, [Action], ActionStatus, branchNumber, AccountNumber, AccountType
, act.accountDescription, isnull(appstatus.DESCRIPTION, '-- CREATED --') as LastStatus, isnull(appstatus.DAYS, '0') DaysSinceLastStatus, datediff(d, ApplicationDate, getdate()) DaysCreated, InitialUserID, isnull(appstatus.STATUS, '-- MADE --') APPLICATIONSTATUS
from dbo.CAOT_Application app
left outer join dbo.CAOT_AccountType act on app.AccountType = act.AccountTypecode
left outer join [CAOT_GetAllApplicationStatus]() appstatus on app.pkApplication = appstatus.[PKAPPLICATION]
where ( isnull(appstatus.STATUSCODE, 'MADE') not in ( 'CANCELLED', 'DECLINED', 'COMPLETE', 'EXPIRED' ) or @showComplete = 'Y' )
and ( @JustMyQueue <> 0)
and ( InitialUserID = @userID)
and ( @ChannelId is null or ChannelID = @ChannelId or ( @ChannelId = 'CBU' and ChannelID is null and isCAO = 'N' ))
order by SortCol, ApplicationDate desc
option (recompile)
end;
That may searching fewer rows of your CAOT_Application table.
Main area of concern
- Missing SET NOCOUNT ON in proc.
- Do not use MTVF :Your UDF is Multi statement TVF(MTVF) Currently it unnecessarily fetch all rows.It give wrong estimate to optimizer.
- MTVF is not require. Create Inline TVF(ITVF) or use Outer Apply.If you use Inline TVF then use them inside OUTER APPLY instead of LEFT JOIN.Pass pkApplicationStatus and @showComplete in Inline TVF as parameter.This will guarantee return only require number of rows.Also ITVF t will execute less than MTVF.
- Alternatively you write ITVF logic in temp table.It will also work ok.
- Condition using too much OR condition are bad perform-ant.
(@JustMyQueue = 0 OR InitialUserID = @userID)
. Try to convert them in UNION ALL or Put them in #temp table and JOIN.
Example,
create table #Status(Status varchar(15))
if(@showComplete='Y')
begin
insert into #Status
end
else
begin
insert into #Status
end
Now simply join #Status
Edit 1 :
You must have notice the performance improvement after passing Parameter in your UDF.
It can be further improve. If possible reduce RETURNS VARCHAR(100) to VARCHAR(50) or so.
ALTER FUNCTION [dbo].[CAOT_GetApplicationStatus]
(@pkApplication INT, @returnType varchar(20))
RETURNS VARCHAR(100)
AS
--DECLARE @status VARCHAR(100)
RETURN(
SELECT TOP 1
CASE @returntype
WHEN 'KEY' THEN CONVERT(VARCHAR(12), isnull(evt.fkApplicationStatus,'1'))
WHEN 'DESCRIPTION' THEN isnull(evt.EventDescription,'-- CREATED --')
WHEN 'CODE' THEN isnull(evt.EventCode,'CREATE')
WHEN 'DAYS' THEN CONVERT(VARCHAR(20), isnull(DATEDIFF(d, ev.createdate, GETDATE()),'0') )
WHEN 'STATUS' THEN isnull(apps.StatusDescription,'-- MADE --')
WHEN 'STATUSCODE' THEN isnull(apps.StatusCode,'MADE')
ELSE isnull(evt.EventDescription,'')
END
FROM dbo.CAOT_ApplicationEvent (NOLOCK) ev
INNER JOIN dbo.CAOT_EventType (NOLOCK) evt
ON ev.fkEventType = evt.pkEventType
INNER JOIN dbo.CAOT_ApplicationStatus (NOLOCK) apps
ON evt.fkApplicationStatus = apps.pkApplicationStatus
WHERE fkApplication = @pkApplication
ORDER BY ev.CreateDate DESC, ev.pkApplicationEvent DESC
)
GO
Don’t forget the #Status join trick.
You should update your script and query plan.