It will only work as intended for single-row updates
The fact that you use TOP 1
and SCOPE_IDENTITY()
tells me this was designed with one-row updates/inserts in mind. But what happens when you update multiple rows? (In case you don't know, the trigger only fires once for the whole changed set, rather than once for each row. The changed rows are added to the inserted
and deleted
tables).
To solve this, I'd recommend using a temp table or variable table instead of @CMP
, and the output
clause instead of @NEWID
. For example:
-- Example table with identity
create table Things (KeyId int identity(1, 1), Value varchar(31))`
-- Table to store identity ints which get created
declare @NewIds table (NewKeyId int)
insert into Things (Value)
output inserted.KeyId into @Keys (NewKeyId) -- this line gets the new identities
select 'Value text'
from OtherTable
Now you can access the created IDs using @Keys
. Of course you'd want to store the EpisodeId
or something in the same table, to relate them.
It's a trigger...
You may already know, but the SQL Server community is fairly divided when it comes to whether triggers should be avoided or not. They can be easy to forget, hard to debug, and can hinder performance as they become part of the write operation.
Since you mention performance I'll elaborate on that aspect, and I'll make some assumptions I hope you can forgive. I'm going to assume there's an application that sits in front of this database, and the application prompts the insert/update. Say the user is on a form and makes changes to an episode, and submits the change. Usually the application will wait for confirmation of the update/insert operation before proceeding (before loading the next page, for instance). With the trigger, the application now has to wait for the initial insert/update, as well as the trigger to complete its operation too.
Therefore, if you haven't already, consider the possibility of separating out this process. In this example I gave above, you could have it so that the application still waits for the initial insert/update operations, loading the next page normally, but then in the background (asynchronously) it tells the database to perform audit operations. Boom, you've reduced page load time, better UX. This also has the benefit of you not having to worry so much about performance tuning the auditing operation.
Checksum?
I'm going to assume since performance is an issue that you've considered indexes, and have an index set up to optimise this Episode
and EpisodeHistory
comparison. Have you considered the use of CHECKSUM()
? I don't have any experience with it myself, however I'd imagine you could use it in combination with persistent computed columns to improve the load when write and maybe reading to the indexes.
In this instance you'd have an extra column on both tables called something like eDescriptionChecksum
, which would be a checksum of eDescription, and you'd put that column in the index instead of eDescription
. Then when you perform the comparison to see if they're the same, you'd be comparing the relatively small and light int
checksum column instead of the potentially-huge (n)varchar
column.
Minor issue
I see when you set the @CMP
, you select from multiple tables. It's recommended to use joins instead – use of multiple tables in the from
clause is being depreciated by all DBMSs.
If anything's unclear, let me know. Hope you found this helpful.