Alternative set-based option for this cursor in SQL?

Question

I am one of those developers that struggle with thinking in terms of sets. At least I know it. :-)

I have been asked to look into the matter of transactional emails being sent out to users. I am not the original programmer of the functionality, and I think the original programmers have left the company as well. Sigh...

Currently, some of the emails are being sent directly out of SQL, but not with Database Mail. So, I am in the process of looking into using Database Mail (sp_send_dbmail). I'll probably do a complete overhaul of how things work, hopefully using Database Mail, but in the meantime, I am wondering whether there is a simpler (temporary way) of improving performance by tinkering with the current SQL code.

In a stored procedure, a cursor is used to call another stored procedure, which makes an API call to a Controller class, which with a few more calls to other methods eventually sends the email. These C# methods are not implemented asynchronously. I'm busy changing those methods to async too. Not surprisingly, the SQL server is half-dead when these emails are being sent out.

So I'm wondering:

1. Whether there is an alternative set-based way instead of using a cursor in this case?
2. Like I said, I'm going to change the C# methods to async in any case (since other parts of the system use it) but off the cuff, would it make a noticeable change in performance? My fairly ignorant opinion would be that the main trouble lies more with the cursor.

Here is the stored procedure with the cursor, sp_Interface_PayslipEmails:

DROP PROCEDURE [dbo].[sp_Interface_PayslipEmails]
GO

SET ANSI_NULLS ON
GO

SET QUOTED_IDENTIFIER ON
GO

CREATE PROC [dbo].[sp_Interface_PayslipEmails](@Periods VARCHAR(MAX),@Resourcetag INT = 0)
AS
BEGIN
DECLARE @Collection VARCHAR(50)
DECLARE @Periodid INT
DECLARE @PortalURL VARCHAR(500) = (SELECT TOP 1 Value from [System Variables] WHERE [Variable] =  'Portal URL')
DECLARE @PeriodTable TABLE ([Period id] INT )
IF @Resourcetag = 0 SET @Resourcetag = NULL
INSERT INTO @PeriodTable
(
    [Period id]
)
select [Value] FROM dbo.fn_Split(@Periods,',') 

SET @Periodid = (SELECT MAX([period id]) FROM @PeriodTable)

SET @Collection = (SELECT dbo.fn_GetCalendarCollectionfromPeriod(@Periodid))
SET @Collection = UPPER(REPLACE(@Collection,'Payrun ',''))

UPDATE I
SET I.[Status] = 'Processed'
FROM [dbo].[PER REM Infoslip] I
INNER JOIN @PeriodTable P
ON I.[Period id] = P.[Period id]
INNER JOIN [dbo].[Calendar Periods] cp
ON P.[Period id]  = cp.[Period ID]
AND [cp].[RunType] = 'Normal'
AND I.[Resource Tag] =ISNULL(@Resourcetag,I.[Resource Tag])
 

SELECT DISTINCT I.[Resource Tag],[I].[E-mail Address] 
INTO #Employees
FROM [dbo].[PER REM Infoslip] I
INNER JOIN @PeriodTable P
ON I.[Period id] = P.[Period id]
INNER JOIN [dbo].[Calendar Periods] cp
ON P.[Period id]  = cp.[Period ID]
AND [cp].[RunType] = 'Normal' --only normal periods available for now
WHERE ISNULL([I].[E-mail Address],'') != ''
AND I.[status] = 'Processed'
AND I.[Resource Tag] = ISNULL(@Resourcetag,I.[Resource Tag])

/* declare variables */
DECLARE @RT INT
DECLARE @EmailAddress VARCHAR(150)
DECLARE @Message VARCHAR(MAX) =''

IF @Collection LIKE '%Feb%2020%'
SET @Message = 'Friendly Reminder. All excess leave will be forfeited on the 28th February as per previous communications.'

DECLARE cursor_name CURSOR FOR SELECT [Resource Tag],[E-mail Address] FROM #Employees
OPEN cursor_name

FETCH NEXT FROM cursor_name INTO @RT,@EmailAddress

WHILE @@FETCH_STATUS = 0
BEGIN
    --API Call to send email
    PRINT @Collection
    PRINT @PortalURL
        EXEC Sp_SendPayslipEmail @PortalURL,@RT,@EmailAddress,@Collection,@Message

    FETCH NEXT FROM cursor_name INTO @RT,@EmailAddress
END

CLOSE cursor_name
DEALLOCATE cursor_name
END 

GO

For more clarity I have included the second stored procedure, Sp_SendPayslipEmail's code here:

DROP PROCEDURE [dbo].[Sp_SendPayslipEmail]
GO

SET ANSI_NULLS ON
GO

SET QUOTED_IDENTIFIER ON
GO

CREATE PROCEDURE [dbo].[Sp_SendPayslipEmail](
  @Hostname NVARCHAR(MAX) 
, @ResourceTag INT
, @Email VARCHAR(256)
, @Period     NVARCHAR(256)
, @Message VARCHAR(MAX))
AS

DECLARE @now VARCHAR(MAX) =  CONVERT(NVARCHAR(MAX), GETDATE(), 13)

DECLARE @OBJECT INT
DECLARE @RESPONSETEXT VARCHAR(8000)
DECLARE @URL NVARCHAR(MAX) =  'https://' + @Hostname + '/payslip/sendpayslipemail?resourceTag=' + CAST(@ResourceTag AS VARCHAR(50))+ '&emailAddress=' + @Email  + '&period=' + @Period + '&message=' + ISNULL(@Message,'') + '&_=' + @now


EXEC sp_OACreate 'Msxml2.ServerXMLHTTP.6.0', @OBJECT OUT
EXEC sp_OAMethod @OBJECT, 'setTimeouts', NULL, 5000, 5000, 30000, 300000

EXEC sp_OAMethod @OBJECT, 'open', NULL, 'get', @URL, 'false'
EXEC sp_OAGetErrorInfo @object
EXEC sp_OAMethod @OBJECT, 'send'
EXEC sp_OAGetErrorInfo @object
EXEC sp_OAMethod @OBJECT, 'responseText',@RESPONSETEXT OUTPUT
SELECT @RESPONSETEXT
EXEC sp_OADestroy @OBJECT
EXEC sp_OAGetErrorInfo @object

GO

UPDATE

I have moved the cursor out of the stored proc InterfacePayslipEmails and into the stored proc SendPayslipEmail. A TVP was also implemented. SendPayslipEmail makes use of the stored procedure sp_send_dbmail and no longer the API call to another part of our system to send the mail.

I have tested that I can receive emails.

Here are the changed stored procedures:

Alter PROC [dbo].[Interface_PayslipEmails](@Periods VARCHAR(MAX),@Resourcetag INT = 0)
AS
BEGIN
    DECLARE @Collection VARCHAR(50)
    DECLARE @Periodid INT
    DECLARE @PortalURL VARCHAR(500) = (SELECT TOP 1 Value from [ System Variables] WHERE [Variable] =  'Portal URL')
    DECLARE @PeriodTable TABLE ([Period id] INT )
    IF @Resourcetag = 0 SET @Resourcetag = NULL
    INSERT INTO @PeriodTable
    (
        [Period id]
    )
    select [Value] FROM dbo.fn_Split(@Periods,',') 
    
    SET @Periodid = (SELECT MAX([period id]) FROM @PeriodTable)

    SET @Collection = (SELECT dbo.fn_GetCalendarCollectionfromPeriod(@Periodid))
    SET @Collection = UPPER(REPLACE(@Collection,'Payrun ',''))

    UPDATE I
    SET I.[Status] = 'Processed'
    FROM [dbo].[PER REM Infoslip] I
    INNER JOIN @PeriodTable P
    ON I.[Period id] = P.[Period id]
    INNER JOIN [dbo].[Calendar Periods] cp
    ON P.[Period id]  = cp.[Period ID]
    AND [cp].[RunType] = 'Normal'
    AND I.[Resource Tag] =ISNULL(@Resourcetag,I.[Resource Tag])
     

    SELECT DISTINCT I.[Resource Tag],[I].[E-mail Address] 
    INTO #Employees
    FROM [dbo].[PER REM Infoslip] I
    INNER JOIN @PeriodTable P
    ON I.[Period id] = P.[Period id]
    INNER JOIN [dbo].[Calendar Periods] cp
    ON P.[Period id]  = cp.[Period ID]
    AND [cp].[RunType] = 'Normal' --only normal periods available for now
    WHERE ISNULL([I].[E-mail Address],'') != ''
    AND I.[status] = 'Processed'
    AND I.[Resource Tag] = ISNULL(@Resourcetag,I.[Resource Tag])

    
/* declare variables */
--DECLARE @RT INT
--DECLARE @EmailAddress VARCHAR(150)
DECLARE @Message VARCHAR(MAX) =''

IF @Collection LIKE '%Feb%2020%'
SET @Message = 'Friendly Reminder. All excess leave will be forfeited on the 28th February as per previous communications.'


--declare variable for Table type
DECLARE @EmailTVP AS EmailAddressTableType;
----insert 
insert into @EmailTVP([resource tag],[e-mail address])
select [resource tag],[e-mail address] from #employees
       
    EXEC SendPayslipEmail @EmailTVP,@PortalURL,@Collection,@Message
    
END 


GO

The updated SendPayslipEmail:

Alter Proc [dbo].[SendPayslipEmail] 
(@TVP EmailAddressTableType READONLY,
@PortalURL VARCHAR(500),
@Collection VARCHAR(50),
@Message VARCHAR(MAX))

as

begin

DECLARE @ResTag INT
DECLARE @Email_Address VARCHAR(150)

DECLARE cursorSendEmail CURSOR
    FOR SELECT [Resource Tag],[E-mail Address] FROM @TVP;
    
OPEN cursorSendEmail  

FETCH NEXT FROM cursorSendEmail INTO @ResTag,@Email_Address;

WHILE @@FETCH_STATUS = 0
BEGIN

 EXEC msdb.dbo.sp_send_dbmail  
    @profile_name = 'PayslipEmails',  
    @recipients = @Email_Address,  
    @body =  @ResTag,  
    @subject = 'Testing DBMail' ;  
        

FETCH NEXT FROM cursorSendEmail INTO @ResTag,@Email_Address;
END

CLOSE cursorSendEmail
DEALLOCATE cursorSendEmail

end

Answer 1

Take back up of Sp_SendPayslipEmail, you need to Alter both proc so decide to create new proc with diffrent name or just Alter it .

There is 2 way :

1. Do Sp_SendPayslipEmail thing in inside sp_Interface_PayslipEmails itself.

In this approach SET Base Query will be different depending what Sp_SendPayslipEmail is doing.

2. Use Table Value Paramters.

Create User define Table Type

CREATE TYPE EmailAddressTableType 
   AS TABLE
      ( RT int
      , EmailAddress VARCHAR(150) );
GO

Then Alter Sp_SendPayslipEmail

Alter proc Sp_SendPayslipEmail 
@TVP EmailAddressTableType READONLY,
--Other paramter
@PortalURL,
@RT,
@EmailAddress,
@Collection,
@Message

as

begin
 print 'do your thing'
end

Finally sp_Interface_PayslipEmails will look like this,

Alter PROC [dbo].[sp_Interface_PayslipEmails](@Periods VARCHAR(MAX),@Resourcetag INT = 0)
AS
BEGIN
DECLARE @Collection VARCHAR(50)
DECLARE @Periodid INT
DECLARE @PortalURL VARCHAR(500) = (SELECT TOP 1 Value from [System Variables] WHERE [Variable] =  'Portal URL')
DECLARE @PeriodTable TABLE ([Period id] INT )
IF @Resourcetag = 0 SET @Resourcetag = NULL
INSERT INTO @PeriodTable
(
    [Period id]
)
select [Value] FROM dbo.fn_Split(@Periods,',') 

SET @Periodid = (SELECT MAX([period id]) FROM @PeriodTable)

SET @Collection = (SELECT dbo.fn_GetCalendarCollectionfromPeriod(@Periodid))
SET @Collection = UPPER(REPLACE(@Collection,'Payrun ',''))

UPDATE I
SET I.[Status] = 'Processed'
FROM [dbo].[PER REM Infoslip] I
INNER JOIN @PeriodTable P
ON I.[Period id] = P.[Period id]
INNER JOIN [dbo].[Calendar Periods] cp
ON P.[Period id]  = cp.[Period ID]
AND [cp].[RunType] = 'Normal'
AND I.[Resource Tag] =ISNULL(@Resourcetag,I.[Resource Tag])


SELECT DISTINCT I.[Resource Tag],[I].[E-mail Address] 
INTO #Employees
FROM [dbo].[PER REM Infoslip] I
INNER JOIN @PeriodTable P
ON I.[Period id] = P.[Period id]
INNER JOIN [dbo].[Calendar Periods] cp
ON P.[Period id]  = cp.[Period ID]
AND [cp].[RunType] = 'Normal' --only normal periods available for now
WHERE ISNULL([I].[E-mail Address],'') != ''
AND I.[status] = 'Processed'
AND I.[Resource Tag] = ISNULL(@Resourcetag,I.[Resource Tag])

/* declare variables */
--DECLARE @RT INT
--DECLARE @EmailAddress VARCHAR(150)
DECLARE @Message VARCHAR(MAX) =''

IF @Collection LIKE '%Feb%2020%'
SET @Message = 'Friendly Reminder. All excess leave will be forfeited on the 28th February as per previous communications.'


--declare variable for Table type
DECLARE @EmailTVP AS EmailAddressTableType;
--insert 
insert into @EmailTVP(RT,EmailAddress)
select RT,EmailAddress from #Employees
    --API Call to send email
    --PRINT @Collection
    --PRINT @PortalURL

    --Done
        EXEC Sp_SendPayslipEmail @EmailTVP,@PortalURL,@Collection,@Message
END 

Get idea about Table Value Paramter before changing,

Table-Valued Parameters

It is very easy.

Your proc has lot of other scope of optimizing but one at a time.

How many mail will be send at one time ??

Edit 1 : sp_OA* objects are obsolete.Configure Dayabase Mail on your server.then use sp_send_dbmail

Edit 2 : See EXEC msdb.dbo.sp_send_dbmail cannot be use In SET Based Approach (without loop).Ypu have to involve Loop like While or Cursor,prefer Cursor.

Using Cursor inside Sp_SendPayslipEmail is far far better than using inside sp_Interface_PayslipEmails.

In case of sp_Interface_PayslipEmails cursor is still open while you are executing

EXEC Sp_SendPayslipEmail @PortalURL,@RT,@EmailAddress,@Collection,@Message

Cursor is open for much longer period.Cursor is expendsive, it consume lot of memory.

So use Cursor inside Sp_SendPayslipEmail .Use TVP record to loop.

We can compemsate for using Cursor be optmizing the proc in some other way.

Note : Do not prefix Store Procedure with 'sp_' hereafter.