SQL Server 2017 Stored Proc & TVF Performance Problem

Posted on

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

  1. Missing SET NOCOUNT ON in proc.
  2. Do not use MTVF :Your UDF is Multi statement TVF(MTVF) Currently it unnecessarily fetch all rows.It give wrong estimate to optimizer.
  3. 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.
  4. Alternatively you write ITVF logic in temp table.It will also work ok.
  5. 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.

Leave a Reply

Your email address will not be published.