Identity and Scope_Identity with triggers

  • So I ran into an issue today and I'm struggling with a solution.

    We have a primary database and an archive database

    In the Primary database exists a table A

    It has an identity column, plus a few additional columns

    Every table in the primary database also have the following columns added during the creation of the DB

    Audit_id column

    DateLastMaint

    PersNbrLastMaint

    These 3 columns are added to every table

    Every table has add, update, delete, SP's

    These were auto generated

    And every table has Audit trigger's for Insert, Update and Delete

    They are INSTEAD OF triggers

    For example, table would look like

    CREATE TABLE [dbo].[JobId](

    [JobIdNbr] [bigint] IDENTITY(1,1) NOT FOR REPLICATION NOT NULL,

    [JobIdValue] [bigint] NULL,

    [InactiveDateTime] [datetime] NULL,

    [DateLastMaint] [datetime] NOT NULL,

    [PersNbrLastMaint] [int] NULL,

    [AuditId] [int] NULL,

    CONSTRAINT [PK_JobId] PRIMARY KEY CLUSTERED

    (

    [JobIdNbr] ASC

    )WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]

    ) ON [PRIMARY]

    So the add SP for this table would look like

    SET NOCOUNT ON

    BEGIN TRY

    SELECT @DateLastMaint = getutcdate()

    INSERT INTO [dbo].[JobId]

    (

    JobIdValue,

    InactiveDateTime,

    DateLastMaint

    )

    VALUES

    (

    @JobIdValue,

    @InactiveDateTime,

    @DateLastMaint

    )

    Begin

    SET @JobIdNbr=@@IDENTITY

    SET @OutRowCount = @@RowCount

    SET @OutErrorNbr = 0

    SET @OutErrorMsg = ''

    End

    RETURN @OutRowCount

    END TRY

    BEGIN CATCH

    SELECT @OutErrorNbr = ERROR_NUMBER(),

    @OutErrorMsg = ERROR_MESSAGE(),

    @OutRowCount = -1

    RETURN -1

    END CATCH

    The instead of trigger fires and actual does the insert

    ALTER TRIGGER [dbo].[trg_AuditInsert_JobId]

    ON [dbo].[JobId]

    INSTEAD OF INSERT

    AS

    BEGIN

    SET NOCOUNT ON;

    -- Update the id and host that is doing the update.

    DECLARE @AuditId int

    EXEC Archive.GetAddAuditId @AuditId OUTPUT

    DECLARE @identCol INT

    SELECT @identCol = JobIdNbr FROM inserted

    IF COALESCE(@identCol,0) > 0

    BEGIN

    SET IDENTITY_INSERT JobId ON

    INSERT INTO dbo.JobId

    (JobIdNbr,JobIdValue,InactiveDateTime,DateLastMaint,AuditId, PersNbrLastMaint)

    SELECT i.JobIdNbr, i.JobIdValue,i.InactiveDateTime,i.DateLastMaint

    , @AuditId

    , CONVERT(BIGINT, CONVERT(BINARY(8), CONTEXT_INFO()))

    FROM inserted i;

    SET IDENTITY_INSERT JobId OFF

    END

    ELSE

    BEGIN

    SET IDENTITY_INSERT JobId OFF

    INSERT INTO dbo.JobId

    (JobIdValue,InactiveDateTime,DateLastMaint,AuditId, PersNbrLastMaint)

    SELECT i.JobIdValue,i.InactiveDateTime,i.DateLastMaint

    , @AuditId

    , CONVERT(BIGINT, CONVERT(BINARY(8), CONTEXT_INFO()))

    FROM inserted i;

    SELECT Inserted.JobIDNbr FROM Inserted;

    END

    END

    Now we added this archive database and also created triggers for it on this table

    ALTER TRIGGER [dbo].[trg_ArchiveJobId_Insert] ON [dbo].[JobId]

    FOR INSERT AS

    BEGIN TRY

    UPDATE [APF_OLTP_Archive].Archive.JobId

    SET Audit_CurrRec = 0

    FROM inserted i

    JOIN [APF_OLTP_Archive].Archive.JobId a

    ON a.JobIdNbr=i.JobIdNbr

    WHERE a.Audit_CurrRec = 1

    INSERT INTO [APF_OLTP_Archive].Archive.JobId

    (

    Audit_CreateDate

    ,Audit_CurrRec

    ,Audit_ActionCd

    ,JobIdNbr

    ,JobIdValue

    ,InactiveDateTime

    ,DateLastMaint

    ,PersNbrLastMaint

    ,AuditId)

    SELECT

    GETUTCDATE() AS Audit_CreateDate

    ,1 AS Audit_CurrRec

    ,'I' AS Audit_ActionCd

    ,i.JobIdNbr

    ,i.JobIdValue

    ,i.InactiveDateTime

    ,i.DateLastMaint

    ,i.PersNbrLastMaint

    ,i.AuditId

    FROM inserted i;

    END TRY

    BEGIN CATCH

    DECLARE @ErrTime datetime, @rc int

    SET @ErrTime = GetUTCDate()

    EXECUTE @rc = [Archive].[AddArchiveLog] @tag='Trigger Creation Error' ,@LogMessage='Error creating Triggers for table: JobId'

    IF @rc > 0 RAISERROR('Error writing to log while creating Trigger for table: JobId',16,1)

    END CATCH

    The problem now is the @@IDENTITY in the main add SP is returning the identity from the Archive Table in the Archive Database

    If I change the line

    SET @JobIdNbr=@@IDENTITY

    to

    SET @JobIdNbr=SCOPE_IDENTITY()

    The returned value is NULL

    Because the insert actually occurred within the INSTEAD OF Trigger.

    So I'm not sure how to fix this.

    Any suggestions would be appreciated.

    Thanks Joe.

  • I think the output clause might work for you.

    http://msdn.microsoft.com/en-us/library/ms177564.aspx"> http://msdn.microsoft.com/en-us/library/ms177564.aspx

    Triggers

    Columns returned from OUTPUT reflect the data as it is after the INSERT, UPDATE, or DELETE statement has completed but before triggers are executed.

    For INSTEAD OF triggers, the returned results are generated as if the INSERT, UPDATE, or DELETE had actually occurred, even if no modifications take place as the result of the trigger operation. If a statement that includes an OUTPUT clause is used inside the body of a trigger, table aliases must be used to reference the trigger inserted and deleted tables to avoid duplicating column references with the INSERTED and DELETED tables associated with OUTPUT.

    DECLARE @JobID TABLE (JobIdNbr INT)

    INSERT INTO [dbo].[JobId]

    (

    JobIdValue,

    InactiveDateTime,

    DateLastMaint

    )

    OUTPUT inserted.JobIdNbr

    INTO @JobID

    SELECT

    @JobIdValue,

    @InactiveDateTime,

    @DateLastMaint

    SELECT @JobIdNbr = (SELECT JobIdNbr FROM @JobID)

  • I tried using the OUTPUT clause but the value returned is 0

    Remember, the code being executed is a SP from C#

    it's called addJobId

    This is where I put the OUTPUT clause

    So now the SP looks like

    CREATE PROCEDURE [dbo].[addJobId]

    (

    @JobIdNbr BIGINT = NULL OUTPUT,

    @JobIdValue BIGINT = NULL,

    @InactiveDateTime DATETIME = NULL,

    @DateLastMaint DATETIME = NULL OUTPUT,

    @OutRowCount INT = 0 OUTPUT,

    @OutErrorNbr INT = 0 OUTPUT,

    @OutErrorMsg VARCHAR(256) = NULL OUTPUT

    )

    AS

    SET NOCOUNT ON

    BEGIN TRY

    SELECT @DateLastMaint = getutcdate()

    DECLARE @JobID TABLE (JobIdNbr INT)

    INSERT INTO [dbo].[JobId]

    (

    JobIdValue,

    InactiveDateTime,

    DateLastMaint

    )

    OUTPUT inserted.JobIdNbr

    INTO @JobID

    SELECT

    @JobIdValue,

    @InactiveDateTime,

    @DateLastMaint

    Begin

    SELECT @JobIdNbr = (SELECT JobIdNbr FROM @JobID)

    SET @OutRowCount = @@RowCount

    SET @OutErrorNbr = 0

    SET @OutErrorMsg = ''

    End

    RETURN @OutRowCount

    END TRY

    BEGIN CATCH

    SELECT @OutErrorNbr = ERROR_NUMBER(),

    @OutErrorMsg = ERROR_MESSAGE(),

    @OutRowCount = -1

    RETURN -1

    END CATCH

    The instead of trigger is

    CREATE TRIGGER [dbo].[trg_AuditInsert_JobId]

    ON [dbo].[JobId]

    INSTEAD OF INSERT

    AS

    BEGIN

    SET NOCOUNT ON;

    -- Update the id and host that is doing the update.

    DECLARE @AuditId int

    EXEC Archive.GetAddAuditId @AuditId OUTPUT

    DECLARE @identCol INT

    SELECT @identCol = JobIdNbr FROM inserted

    IF COALESCE(@identCol,0) > 0

    BEGIN

    SET IDENTITY_INSERT JobId ON

    INSERT INTO dbo.JobId

    (JobIdNbr,JobIdValue,InactiveDateTime,DateLastMaint,AuditId, PersNbrLastMaint)

    SELECT i.JobIdNbr, i.JobIdValue,i.InactiveDateTime,i.DateLastMaint

    , @AuditId

    , CONVERT(BIGINT, CONVERT(BINARY(8), CONTEXT_INFO()))

    FROM inserted i;

    SET IDENTITY_INSERT JobId OFF

    END

    ELSE

    BEGIN

    SET IDENTITY_INSERT JobId OFF

    INSERT INTO dbo.JobId

    (JobIdValue,InactiveDateTime,DateLastMaint,AuditId, PersNbrLastMaint)

    SELECT i.JobIdValue,i.InactiveDateTime,i.DateLastMaint

    , @AuditId

    , CONVERT(BIGINT, CONVERT(BINARY(8), CONTEXT_INFO()))

    FROM inserted i;

    SELECT Inserted.JobIDNbr FROM Inserted;

    END

    END

    These work fine as log as we do not turn archiving on for this given table.

    If we turn archiving on, another trigger is dynamically created and added to this table to write the data to the archive database

    CREATE TRIGGER [dbo].[trg_ArchiveJobId_Insert] ON [dbo].[JobId]

    FOR INSERT AS

    BEGIN TRY

    UPDATE [APF_OLTP_Archive].Archive.JobId

    SET Audit_CurrRec = 0

    FROM inserted i

    JOIN [APF_OLTP_Archive].Archive.JobId a

    ON a.JobIdNbr=i.JobIdNbr

    WHERE a.Audit_CurrRec = 1

    INSERT INTO [APF_OLTP_Archive].Archive.JobId

    (

    Audit_CreateDate

    ,Audit_CurrRec

    ,Audit_ActionCd

    ,JobIdNbr

    ,JobIdValue

    ,InactiveDateTime

    ,DateLastMaint

    ,PersNbrLastMaint

    ,AuditId)

    SELECT

    GETUTCDATE() AS Audit_CreateDate

    ,1 AS Audit_CurrRec

    ,'I' AS Audit_ActionCd

    ,i.JobIdNbr

    ,i.JobIdValue

    ,i.InactiveDateTime

    ,i.DateLastMaint

    ,i.PersNbrLastMaint

    ,i.AuditId

    FROM inserted i;

    END TRY

    BEGIN CATCH

    DECLARE @ErrTime datetime, @rc int

    SET @ErrTime = GetUTCDate()

    EXECUTE @rc = [Archive].[AddArchiveLog] @tag='Trigger Creation Error' ,@LogMessage='Error creating Triggers for table: JobId'

    IF @rc > 0 RAISERROR('Error writing to log while creating Trigger for table: JobId',16,1)

    END CATCH

  • @@ROWCOUNT must be captured immediately after the SQL statement or its value gets reset.

    However, since you've added an OUTPUT clause, you can get the number of rows affected form that table instead, and that should always be accurate.

    ...

    Begin

    SELECT @JobIdNbr = (SELECT JobIdNbr FROM @JobID)

    SET @OutRowCount = (SELECT COUNT(*) FROM @JobID) --@@ROWCOUNT

    SET @OutErrorNbr = 0

    SET @OutErrorMsg = ''

    End

    ...

    SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".

  • The outputrowcount is fine. It returns 1

    it's my identity column which is returning 0

    I also tried Ident_current('dbo.jobid')

    This returns the correct value

    But, according to MSDN site

    Be cautious about using IDENT_CURRENT to predict the next generated identity value. The actual generated value may be different from IDENT_CURRENT plus IDENT_INCR because of insertions performed by other sessions.

    •IDENT_CURRENT returns the last identity value generated for a specific table in any session and any scope.

    •@@IDENTITY returns the last identity value generated for any table in the current session, across all scopes.

    •SCOPE_IDENTITY returns the last identity value generated for any table in the current session and the current scope.

  • I think you have your architecture a bit backwards???

    Instead of storing the audit id in the primary database tables, why aren't you storing the PK of the tables in the primary DB in the audit tables?

    So, with this, you can use an insert trigger, and use the INSERTED table to insert values into the audit table.

    If you keep the architecture the same, change the trigger to an insert trigger.

    Use the INSERTED table to create the archive, and use scope_identity to back-fill the audit id the newly created row in the primary database.

    Also, why use triggers at all? Why can't your insert proc insert the values into the primary DB and the audit DB?

    Lastly, I'm not sure what your auditing requirements may be, but there are a lot of potential problems with this architecture. All this work being done in triggers may be a performance issue. By nature, the audit should be separate from the data being audited. This appears to be on the same server. I could go further, but without more info, I may not be correct.

    Michael L John
    If you assassinate a DBA, would you pull a trigger?
    To properly post on a forum:
    http://www.sqlservercentral.com/articles/61537/

  • joepacelli (10/8/2014)


    The outputrowcount is fine. It returns 1

    it's my identity column which is returning 0

    I also tried Ident_current('dbo.jobid')

    This returns the correct value

    But, according to MSDN site

    Be cautious about using IDENT_CURRENT to predict the next generated identity value. The actual generated value may be different from IDENT_CURRENT plus IDENT_INCR because of insertions performed by other sessions.

    •IDENT_CURRENT returns the last identity value generated for a specific table in any session and any scope.

    •@@IDENTITY returns the last identity value generated for any table in the current session, across all scopes.

    •SCOPE_IDENTITY returns the last identity value generated for any table in the current session and the current scope.

    It's only "fine" by accident. The SET statement before it always causes @@ROWCOUNT to return 1. That is, as currently written, no matter how many rows are affected by the statement, you'll always get 1 in the row count value:

    DECLARE @id int

    SELECT * FROM sys.objects --note the "(nnn rows processed)" message

    SET @id = 5

    SELECT @@ROWCOUNT

    SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".

  • Yes all these triggers does make it hard to debug. Change Data Capture could be worth looking into.

    Also I noticed in the instead of trigger, the code is on/off but not sure why this is begin done.

    SET IDENTITY_INSERT JobId ON

    SET IDENTITY_INSERT JobId OFF

    This code also captures the IDentity value, but not getting used later.

    SELECT @identCol = JobIdNbr FROM inserted

    Another issue is this trigger assumes that you wiill only have 1 insert at a time going into this table.

  • ScottPletcher (10/8/2014)


    joepacelli (10/8/2014)


    The outputrowcount is fine. It returns 1

    it's my identity column which is returning 0

    I also tried Ident_current('dbo.jobid')

    This returns the correct value

    But, according to MSDN site

    Be cautious about using IDENT_CURRENT to predict the next generated identity value. The actual generated value may be different from IDENT_CURRENT plus IDENT_INCR because of insertions performed by other sessions.

    •IDENT_CURRENT returns the last identity value generated for a specific table in any session and any scope.

    •@@IDENTITY returns the last identity value generated for any table in the current session, across all scopes.

    •SCOPE_IDENTITY returns the last identity value generated for any table in the current session and the current scope.

    It's only "fine" by accident. The SET statement before it always causes @@ROWCOUNT to return 1. That is, as currently written, no matter how many rows are affected by the statement, you'll always get 1 in the row count value:

    DECLARE @id int

    SELECT * FROM sys.objects --note the "(nnn rows processed)" message

    SET @id = 5

    SELECT @@ROWCOUNT

    Which brings up the next issue: the cursor is written to only accomodate single row inserts. If there is ever a time whereby you insert a set of job entries, the cursor logic will fail.

    ----------------------------------------------------------------------------------
    Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?

  • Now that I look at this harder, there are many things that are not going to work very well.

    The insert procedure:

    As Scott Pointed out,

    Begin

    SET @JobIdNbr=@@IDENTITY <-- THIS SHOULD BE SCOPE_IDENTITY(). Yu may not get the desired results with @@IDENTITY

    SET @OutRowCount = @@RowCount <-- THIS IS ALWAYS GOING TO BE ONE, because it refers to the line above ONLY.

    SET @OutErrorNbr = 0

    SET @OutErrorMsg = ''

    End

    Not sure of the point of putting this in a begin / end either, to each hos own, but the line referring to the @@ROWCOUNT needs to be above all of this, directly below the INSERT.

    Don'd do this:

    RETURN @OutRowCount Use an output parameter, or return a result set. A procedure returns 0 if it is successful, non-zero if it fails. This forces a lot of extra code to handle non-default behaviors.

    Where are you controlling transactions? In the code? It's probably a good idea to include a begin/commit/rollback trans in this proc. If it's being handled in code, then OK.

    Unless it was left out of the posting, you should include the lines SET NCOUNT ON and SET XACT_ABORT ON. Google them. They make sense.

    Triggers:

    Really, not to sound harsh, but this is a mess. As others have pointed out, it will only handle a single row being inserted. Multiple rows will not work.

    What is the point of this code?

    DECLARE @identCol INT

    SELECT @identCol = JobIdNbr FROM inserted

    IF COALESCE(@identCol,0) > 0

    COALESCE is not needed of no row exists, @identCol will be null. So, test for NULL.

    Multiple rows will only return the last row.

    Whats the point of this code?

    UPDATE [APF_OLTP_Archive].Archive.JobId

    SET Audit_CurrRec = 0

    FROM inserted i

    JOIN [APF_OLTP_Archive].Archive.JobId a

    ON a.JobIdNbr=i.JobIdNbr

    WHERE a.Audit_CurrRec = 1

    Looking at this, it appears that you have the CurrRec flag set to 1 for the most recent entry, and want to re-set it to 0 when it's no longer the most recent record. That may not be needed. Would a date stamp field work to try to capture the last record?

    Plus, this will update every record in the archive table with that JobIdNbr. Lots of overhead!

    Why are you inserting a row in an insert trigger? Not sure I get that one.

    I retract my previous comment that this may be a performance problem. It WILL be a performance problem.

    And it will be a maintenance nightmare.

    To get to the juicy parts, what exactly are you trying to do? Capture all changes to a row? Whats the purpose of this auditing?

    Because this does not meet HIPAA requirements, probably not SARBOX requirements, and it definitely does not meet any of the financial auditing requirements because there is no separation of data.

    Michael L John
    If you assassinate a DBA, would you pull a trigger?
    To properly post on a forum:
    http://www.sqlservercentral.com/articles/61537/

Viewing 10 posts - 1 through 9 (of 9 total)

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