## 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. ### EDIT: Untested replacement So I've just roughly written some SQL I think should work instead of everything including and below the `SELECT Top 1` statement. The main issue I have with that part of your trigger is that it will only work for one-row updates, and it performs an INSERT then an UPDATE on that inserted data, which can be optimised. INSERT into EpisodeHistory SELECT EpisodeID = d.EpisodeID, eOwner = d.eOwner, eDescription = iif(h.HistoryID is null, d.eDescription, ''), eDescRef = isnull(h.HistoryID, 0), hWhen = getDate(), from deleted d left join ( select HistoryID, EpisodeID, eOwner, eDescription, row_number() over (partition by EpisodeID, eOwner, eDescription ORDER BY HistoryID) as RowNumber from EpisodeHistory where eDescription <> '' ) h on d.EpisodeID = h.EpisodeID and d.eOwner = h.eOwner and d.eDescription = h.eDescription and h.RowNumber = 1 Rather than inserting the EpisodeHistory, *then* checking and updating based on the data we've just inserted, this code just does the operation all in one. First I'll explain the left join. The point of the join is to find the `HistoryID` of the earliest matching `EpisodeHistory` row (since we want to put it in `eDescRef`, right?). So we do a normal select of the `EpisodeHistory` table, except we also add a column for [`row_number()`][3], which, when combined with the fact that we've put `RowNumber = 1` in the join condition, will means it will only join to the first `HistoryID` that matches. This makes sure that if there are two rows with the same `EpisodeID`, `eOwner` and `eDescription`, it will only join to the one with the lowest `HistoryID`. Now the actual columns that are inserted are fairly simple. If the left join fails (if no rows in EpisodeHistory match), selecting `h.HistoryID` will return `NULL`. Therefore, `eDescription = iif(h.HistoryID is null, d.eDescription, '')` means that if a `HistoryID` was not found in the join, it will just do a normal insert of `eDescription`. If one *was* found, it will blank that field. The next line is fairly simple in that it inserts `HistoryID` if the join happened, otherwise it puts in 0. Since the entire operation can be done in one statement like above, so you can ignore that bit I said above about temporary tables and an `output` clause. This code above also has the added bonus of working for multiple-row updates. ## 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()`][1]? I don't have any experience with it myself, however I'd imagine you could use it in combination with [persistent computed columns][2] 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. [1]: https://docs.microsoft.com/en-us/sql/t-sql/functions/checksum-transact-sql [2]: https://docs.microsoft.com/en-us/sql/t-sql/statements/alter-table-computed-column-definition-transact-sql [3]: https://docs.microsoft.com/en-us/sql/t-sql/functions/row-number-transact-sql