October 7, 2014 at 7:52 pm
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.
October 7, 2014 at 9:46 pm
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)
October 8, 2014 at 5:32 am
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
October 8, 2014 at 11:23 am
@@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".
October 8, 2014 at 12:18 pm
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.
October 8, 2014 at 12:51 pm
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/
October 8, 2014 at 1:03 pm
joepacelli (10/8/2014)
The outputrowcount is fine. It returns 1it'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".
October 8, 2014 at 1:06 pm
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.
October 8, 2014 at 2:35 pm
ScottPletcher (10/8/2014)
joepacelli (10/8/2014)
The outputrowcount is fine. It returns 1it'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?
October 8, 2014 at 3:22 pm
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