Looking for advice on error logging.

  • Hi Folks,

    I've been asked to make some modifications to the nightly data load jobs.  We have had some instances where a staging table was empty and the source table was wiped.  These tables are only refreshed every 24hrs for some dashboards.  I just want to do a simple check to see if the table has any rows before we wipe any data. Currently the procedure has output parameters that the calling application uses to log its own messages. I don't want to break this process but I want to raise my own error so I can get an alert if data is missing and log it.  Below is an example of one of the procedures with the IF ELSE block I have added.  I have tested it and it does work as intended but I have this feeling like it's not the best way to go about it.  I'm thinking it might be better to move the IF statement inside by TRY block...  Any Ideas?  I can't change the logging of the application.


    ALTER PROCEDURE [usp_LoadData] @errorcode INT OUTPUT, @messageText VARCHAR(1024) OUTPUT
    AS

    DECLARE @ScriptLog VARCHAR(1024) = ''
    DECLARE @ScriptError INT = 0

    --Check to see if the staging table contains any data
    IF EXISTS (SELECT TOP 1 1 FROM Staging.dbo.Sometable)

    BEGIN TRY

        -- Begin Data load:
        TRUNCATE TABLE Sometable

        INSERT INTO Sometable
        (ID
        ,LastName)

        SELECT ID
        ,LastName
        FROM [Staging].dbo.Sometable
        

        SET @ScriptLog = @ScriptLog + '<line>' + cast(@@ROWCOUNT AS VARCHAR(1024)) + ' records added.</line>'

    END TRY

    BEGIN CATCH
      SET @ScriptError = ERROR_NUMBER()
        SET @ScriptLog = @ScriptLog + '<line>' + left(ERROR_MESSAGE(),128) + '</line>'
    END CATCH

    --Raise error if table is empty
    ELSE
    BEGIN
        SET @ScriptLog = @ScriptLog + '<line>Load canceled because source table is empty.</line>'
        SET @errorcode = 16
        RAISERROR('Table Sometable was not loaded because the staging table was empty', @errorcode, 1) WITH LOG
    ENDSELECT @errorcode = @ScriptError , @messageText = @ScriptLog


    SELECT quote FROM brain WHERE original = 1
    0 rows returned

  • The code looks good to me.  I think it's better to test row existence outside the TRY.

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

  • ScottPletcher - Thursday, September 27, 2018 1:02 PM

    The code looks good to me.  I think it's better to test row existence outside the TRY.

    Thanks for the extra pair of eyes.  Maybe it's just paranoia but sometimes I get the feeling like "no you are doing this wrong, and there's a better way".  😉


    SELECT quote FROM brain WHERE original = 1
    0 rows returned

  • One thing I'd do is set @ScriptLog immediately after the BEGIN TRY with details of the operation you're about to try - that way if it fails your log contains details of what it was trying to do when it failed, which can aid debugging no end.

Viewing 4 posts - 1 through 3 (of 3 total)

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