Stored Procedure help

  • Hi guys, 

    I have been asked to amend a stored procedure created by another colleague as it is not sending out the emails as required. 

    What I am failing to understand is why the first set of code doesn't send out the emails but the second chunk of code does?

    disclaimer: my understanding of Begin transaction, Try and End Catch is basic at best so ideally I'm happy to learn something new.

    thanks in advance 

    **CODE 1** 

    USE [Work]
    GO

    SET ANSI_NULLS ON
    GO
    SET QUOTED_IDENTIFIER ON
    GO

    ALTER Procedure [dbo].[usp_G6Report] AS

    DECLARE @auditstart DATETIME
    DECLARE @auditprocedurename VARCHAR(255)

    SET @auditstart = (SELECT GETDATE())
    SET @auditprocedurename = (SELECT name FROM sys.procedures WHERE object_id = @@procid)

    DECLARE @xml NVARCHAR(MAX)
    DECLARE @body1 NVARCHAR(MAX)
    DECLARE @EmailBody AS VARCHAR(MAX)
    DECLARE @Today DATE = GETDATE()
    DECLARE @Day varchar(10) = 'Monday'

    IF DATENAME(DW,@Today) = @Day
    BEGIN
    --------------------
    BEGIN TRANSACTION [G65Report]

        BEGIN TRY

    SET @xml = CAST((
            select distinct
                td = accountnumber
                ,''
                , td = code
                , ''
                , td = user
                , ''
                ,td = DaysOnBlock
                , ''
                ,td = Portfolio
            from (
    select
     a.accountnumber
    , a.code
    , b.user
    , DATEDIFF(dd,convert(date,Code1Date)
    ,convert(date, getdate())) as DaysOnBlock
    ,c.Portfolio
    from FirstVision.dbo.vw_current a
    left join firstvision.dbo.vw_Current2 b
    on a.accountnumber=b.accountnumber
    left join worklsc.dbo.vw_Portfolios c
    on a.AccountNumber = c.AccountNumber
    where a.code01 = 'G'
    and b.user = '65'
    and DATEDIFF(dd,convert(date,Code1Date),convert(date, getdate())) >30

                ) a
            ORDER BY DaysOnBlock DESC
            FOR XML PATH('tr'), ELEMENTS)
        AS NVARCHAR(MAX))

        
    COMMIT TRANSACTION [G65Report]
        END TRY

        BEGIN CATCH

    ;ROLLBACK TRANSACTION [G65Report]
            
    ;DECLARE @Error varchar(200) = CAST(ERROR_LINE() AS VARCHAR) + CHAR(13) + CAST(ERROR_MESSAGE() AS VARCHAR)

    SET @body1 ='<html><body><H5> G65 Accounts - FreshStart</H5>
    <table border = 1>
    <tr><H5>
    <th>AccountNumber </th><th> code</th><th> user </th><th> DaysOnBlock</th><th> Portfolio</th><th>'
    SET @body1 = @body1 + '<H5>' + @xml + '</H5>' + '</table></body></html>'

    EXEC Work.dbo.usp_EmailProgress
    @Recipients = NULL
    ,@copy_recipients = 'example@company.co.uk'
    ,@Subject = 'G65 report'
    ,@from_address = 'example@company.co.uk'
    ,@reply_to = 'example@company.co.uk'
    ,@body = @body1
    ,@body_format ='HTML'

        END CATCH;
    --------------------
    END

    ELSE
    BEGIN PRINT 1
    END

      
    EXEC work.dbo.usp_audit_procedure @auditprocedurename, @auditstart

    ***CODE 2****

    USE [Work]
    GO

    SET ANSI_NULLS ON
    GO
    SET QUOTED_IDENTIFIER ON
    GO

    ALTER Procedure [dbo].[usp_G65Report] AS

    SET NOCOUNT ON ;

    DECLARE @auditstart DATETIME
    DECLARE @auditprocedurename VARCHAR(255)

    SET @auditstart = (SELECT GETDATE())
    SET @auditprocedurename = (SELECT name FROM sys.procedures WHERE object_id = @@procid)

    DECLARE @Today DATE = GETDATE()
    DECLARE @Day varchar(10) = 'Wednesday'

    IF DATENAME(DW,@Today) = @Day
    BEGIN
    --------------------

    DECLARE @TABLEHTML NVARCHAR (MAX) ;
    SET @TABLEHTML=

    N'<H1><font size = "3" color="black"> Hello, below is the G65 report. </font></H1>' +
    N'<table border="1" style type="text/css">' +
    N'<tr>AccountNumber</th></td>' +
    N'<th>code</th></td>' +
    N'<th>User</th></td>' +
    N'<th>Daysonblock</th></td>' +
    N'<th>Portfolio</th></td>' +

                    
                     CAST((
            select distinct
                 td = accountnumber, '' ,
                 td = code , '' ,
                td = user , '' ,
                 td = DaysOnBlock, '' ,
                 td = Portfolio, ''
            
            from (
    select
            a.accountnumber
            , a.code
            , b.user
            , DATEDIFF(dd,convert(date,Code1Date)
            ,convert(date, getdate())) as DaysOnBlock
            ,c.Portfolio
    from FirstVision.dbo.vw_current a
    left join firstvision.dbo.vw_Current2 b
    on a.accountnumber=b.accountnumber
    left join work.dbo.vw_Portfolios c
    on a.AccountNumber = c.AccountNumber
    where a.code = 'G'
    and b.user = '65'
    and DATEDIFF(dd,convert(date,Code1Date),convert(date, getdate())) >30

                ) a
            ORDER BY DaysOnBlock DESC
            FOR XML PATH('tr'), TYPE)
        AS NVARCHAR(MAX))
            
            
    ;DECLARE @Error varchar(200) = CAST(ERROR_LINE() AS VARCHAR) + CHAR(13) + CAST(ERROR_MESSAGE() AS VARCHAR)

    EXEC Work.dbo.usp_EmailProgress
    @Recipients = 'example@company.co.uk'
    ,@copy_recipients = 'example@company.co.uk '
    ,@reply_to = 'example@company.co.uk'
    ,@Subject = 'G65 '
    ,@body = @TABLEHTML    
    ,@body_format ='HTML'

    --------------------
    END

    ELSE
    BEGIN PRINT 1
    END

      
    EXEC work.dbo.usp_audit_procedure @auditprocedurename, @auditstart

  • At a quick glance, the email in the first SP is only sent if the CATCH is entered. If there are no errors in your TRY, the transaction is committed and the TRY CATCH is exited; the CATCH section is never entered (where your Work.dbo.usp_EmailProgress is). Your second SP has no TRY CATCH, so the whole statement is executed.#

    If you're not sure how TRY CATCH and Transactions work fully, then please ask.

    Thom~

    Excuse my typos and sometimes awful grammar. My fingers work faster than my brain does.
    Larnu.uk

  • I think Thom has already answered your question.  But I'm not sure what the purpose of your stored procedures are, especially the first one.  It sets the value of a variable, and sends an e-mail if there's an error while doing so.  The variable isn't subsequently used.  There's no need for the explicit transaction, either, because there's nothing to commit or roll back.

    John

  • Thought I might as well post a little on TRY CATCH and Transactions with a little example. The actual process here, in the sense of the transaction, is a little pointless here as the whole process is very simple; it's just an example.

    Firstly, the TRY part of the SP in the below example, literally tries to insert a record into the table Client. It operates in a transactions, which is declared at the start of the SP. If the TRY is succesful, the COMMIT is run, which (unsurprisingly) commits the transaction (saving the data to the table). If, however, the INSERT fails, then the TRY is exited and the CATCH is entered. In this example I provide a record which breaks the Valid_DOB constraint (by giving a date in the future). The CATCH statement then "rolls back" the transaction, which means that anything that has happened in side that transaction is effectively undone (it's worth, however, noting that the seed on ID is NOT rolled back). It also returns the error message for the user. Finally the CATCH is ended as well.

    USE Sandbox;
    GO
    --CREATE a quick sample table
    CREATE TABLE Client (ID int IDENTITY(1,1), ClientName varchar(50), DoB date);
    --Add a Constraint so that the DOB cannot be in the Future
    ALTER TABLE Client ADD CONSTRAINT ValidDOB CHECK (Dob < GETDATE());
    GO
    --CREATE an SP to insert data
    CREATE PROC InsertClient (@ClientName varchar(50), @Dob Date) AS
     
      --Start a Transaction
      BEGIN TRANSACTION New_Client;
      --Start the Try
      BEGIN TRY
       PRINT 'Creating new Client ' + @Clientname + '.';
       --Try to insert the data
       INSERT INTO Client (ClientName,DoB)
       VALUES (@ClientName, @Dob);
       --If the above worked, COMMIT
       COMMIT TRANSACTION New_Client
       PRINT @ClientName + ' has been added.';
       --End the TRY
      END TRY
      --Start the Catch, this will be entered if the INSERT failed
      BEGIN CATCH
       --Get the error details
       DECLARE @ERROR VARCHAR(MAX);
       SET @ERROR = ERROR_MESSAGE();
       --Rollback the transaction
       ROLLBACK TRANSACTION New_Client;

       PRINT 'Failed to create new Client ' + @Clientname + '.';
       --return the error
       RAISERROR(@ERROR, 11, -1);    
      END CATCH
    GO

    --Ok, now let us try and add some data. First some good data:
    EXEC InsertClient 'Steve Jobs', '19740105';
    EXEC InsertClient 'Jane Bloggs', '19850508';
    GO
    --Check those records are there:
    SELECT *
    FROM Client;
    GO
    --Now a good and bad record
    EXEC InsertClient 'Rob Beckett', '20171114';
    EXEC InsertClient 'Suzie Wolff', '19861224';
    GO
    --Have a look at your PRINT statements here. You should see a error message, and a PRINT statement saying the INSERT failed.
    --Check the records are there. There should only be 3.
    SELECT *
    FROM Client;

    GO
    --Clean up
    DROP PROC InsertClient;
    DROP TABLE Client;

    Please do ask if any of this doesn't make sense.

    Thom~

    Excuse my typos and sometimes awful grammar. My fingers work faster than my brain does.
    Larnu.uk

  • Thank you both for your explanations. 

    Tom- I'm going to be honest, I can't get my head around how TRY, CATCH and TRANSACTIONS fully work.

    Is there a Lehmans explanation available? 

    thanks in advance

  • Apologies didnt see your post. 

    let me take a read and see where I get.

  • ExhibitA - Wednesday, August 9, 2017 4:52 AM

    Thank you both for your explanations. 

    Tom- I'm going to be honest, I can't get my head around how TRY, CATCH and TRANSACTIONS fully work.

    Is there a Lehmans explanation available? 

    thanks in advance

    Just posted something before your post, which might help you a little. I had a quick Google as well, and found this document, which might help you too: https://www.red-gate.com/simple-talk/sql/database-administration/handling-errors-in-sql-server-2012/

    Thom~

    Excuse my typos and sometimes awful grammar. My fingers work faster than my brain does.
    Larnu.uk

  • ExhibitA - Wednesday, August 9, 2017 4:52 AM

    Is there a Lehmans explanation available? 

    thanks in advance

    Did you perhaps mean a "layman's explanation"?

    Drew

    J. Drew Allen
    Business Intelligence Analyst
    Philadelphia, PA

Viewing 8 posts - 1 through 7 (of 7 total)

You must be logged in to reply to this topic. Login to reply