January 25, 2011 at 2:14 pm
Any thoughts on how to approach a multi-event DML Trigger that fires on INSERT, UPDATE and DELETE where the action taken changes based on the event?
This may be one of those "Whatever floats your boat" kind of answers and if so thats fine but I'd rather ask now then find out later there were possible issues I didn't;t take into account.
If I have to set up a Trigger for all 3 DML actions and the trigger does something different for each of these 3 is it better to design 1 Trigger for all 3 and then within the Trigger execute one of 3 paths based on the event or design 1 Trigger for UPDATE, 1 for INSERTR and 1 for DELETES?
Any thoughts are appreciated.
Thanks
Kindest Regards,
Just say No to Facebook!January 26, 2011 at 1:27 am
I don't think it makes any difference on the performance side, so I would pick a single trigger.
I think it's up to personal preference.
-- Gianluca Sartori
January 26, 2011 at 1:53 am
Personally I'd follow the 'single responsibility' rule. If there is nothing in common between what's done on an insert and what's done on an update, they do not belong in the same trigger. If what's done is mostly the same, then one trigger's probably easier.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
January 27, 2011 at 9:37 am
Well the AFTER INSERT/UPDATE actions will both plug in a date/time value in the Inserted/added row when the field that holds the date/time is left NULL.
The table in question has 4 auditing fields; 2 set to DATETIME and 2 set to NUMERIC. The date/time fields store date/time the row was inserted and or last modified. The application that uses the DB that holds this table doe snot populate any of these 4 fields even though they have been in the table for several years.
I have the necessary logic to plug in the data for the CreatedBy & CreatedWhen fields as well as for the LastModifedBY & LastModifiedOn fields, I just wasn't sure if these should be a single trigger which branches to 1 of 2 updates based on the event being an Insert or an Update or if they should be 2 independent triggers.
Now that you know the details what say yee? 1 trigger or 2?
Thanks
Kindest Regards,
Just say No to Facebook!January 27, 2011 at 9:46 am
If a different action is taken on update than on insert and no code is in common they should be two triggers. Single responsibility
What I'm saying is that if your trigger looks like this, then it should be two triggers
CREATE TRIGGER ...
BEGIN
IF <Insert>
-- Do one thing
IF <Update>
-- Do something else
END
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
January 27, 2011 at 9:56 am
I think GilaMonster made this point, for substantially the same work you should have a single trigger, for essentially different processes different triggers make sense. Also, for the insert, are you using defaults for the dates? that would seem to take care of part of it.
I am torn on whether it is the apps job to fill in these fields in the first place. I'd end up with triggers to make sure it was happening so having the app fill them in really doesn't do much..
CEWII
January 27, 2011 at 9:59 am
Elliott Whitlow (1/27/2011)
I am torn on whether it is the apps job to fill in these fields in the first place. I'd end up with triggers to make sure it was happening so having the app fill them in really doesn't do much..
On inserts there should be defaults to take care of values that aren't specified and need to be set. On updates it should be a trigger in case someone bypasses the app or a new app (or new piece of old app) is written.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
January 27, 2011 at 10:02 am
GilaMonster (1/27/2011)
Elliott Whitlow (1/27/2011)
I am torn on whether it is the apps job to fill in these fields in the first place. I'd end up with triggers to make sure it was happening so having the app fill them in really doesn't do much..On inserts there should be defaults to take care of values that aren't specified and need to be set. On updates it should be a trigger in case someone bypasses the app or a new app (or new piece of old app) is written.
Agreed, given that I don't trust apps to always do the right thing a trigger is always in place, I was just doing this the other day, and I am writing all the code. Basically I want to be sure that any possible later developer can't get it wrong.. (at least not without trying to!)
CEWII
January 27, 2011 at 11:33 am
GilaMonster (1/27/2011)
Elliott Whitlow (1/27/2011)
I am torn on whether it is the apps job to fill in these fields in the first place. I'd end up with triggers to make sure it was happening so having the app fill them in really doesn't do much..On inserts there should be defaults to take care of values that aren't specified and need to be set. On updates it should be a trigger in case someone bypasses the app or a new app (or new piece of old app) is written.
Maybe I missed something but Defaults don't work even for Updates if the INSERT does not list that field in its INSERT, isn't that right?
For example if I have this:
INSERT INTO MYTABLE ( TextFiled1, TextField2, DateField1)
VALUES ('Hello World', 'GoodBye Best Practices', '2011/01/27')
And MYTABLE also has 2 fields not listed in the INSERT (this excludes the auto-incrementing PK) then even if I plug in a Default for them they will still be set to NULL because the INSERT doesn't list those fields. Isn't that right?
Like I said I may just have misunderstood this but I was taught that Defaults only work when the INSERT does not explicitly specify NULL for the field (this assumes its a Nullable field which this apps database is full of) and when the INSERT lists the field and uses DEFAULT for the insert to specify the use of the fields default value or formula.
Am I wrong?
Kindest Regards,
Just say No to Facebook!January 27, 2011 at 11:35 am
Elliott Whitlow (1/27/2011)
GilaMonster (1/27/2011)
Elliott Whitlow (1/27/2011)
I am torn on whether it is the apps job to fill in these fields in the first place. I'd end up with triggers to make sure it was happening so having the app fill them in really doesn't do much..On inserts there should be defaults to take care of values that aren't specified and need to be set. On updates it should be a trigger in case someone bypasses the app or a new app (or new piece of old app) is written.
Agreed, given that I don't trust apps to always do the right thing a trigger is always in place, I was just doing this the other day, and I am writing all the code. Basically I want to be sure that any possible later developer can't get it wrong.. (at least not without trying to!)
CEWII
To give you an idea of the Joy of thid apps DB design I'd thought I'd share with you the follwoing bit of trivial info. Not one instance of an audting field in any table be it the Date/Time field or the UserID field is set to NOT ALLOW NULLS. Hows that for Best Practices design?
Kindest Regards,
Just say No to Facebook!January 27, 2011 at 11:45 am
YSLGuru (1/27/2011)
Maybe I missed something but Defaults don't work even for Updates if the INSERT does not list that field in its INSERT, isn't that right?
Defaults never work for updates
For example if I have this:
INSERT INTO MYTABLE ( TextFiled1, TextField2, DateField1)
VALUES ('Hello World', 'GoodBye Best Practices', '2011/01/27')
And MYTABLE also has 2 fields not listed in the INSERT (this excludes the auto-incrementing PK) then even if I plug in a Default for them they will still be set to NULL because the INSERT doesn't list those fields. Isn't that right?
No, that's exactly the case where the default will work
Like I said I may just have misunderstood this but I was taught that Defaults only work when the INSERT does not explicitly specify NULL for the field (this assumes its a Nullable field which this apps database is full of) and when the INSERT lists the field and uses DEFAULT for the insert to specify the use of the fields default value or formula.
Defaults only work when the column is not mentioned at all in the insert statement or when DEFAULT is specified. It will not work if the insert explicitly states NULL for the column value. That situation is treated exactly like any other explicit value.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
January 27, 2011 at 12:47 pm
Like Gail, I'd be inclined to split the triggers up. SQL Server will want to have execution plans for them, and multiple internal branches generally result in expensive plans. You want triggers to finish as rapidly as possible so as to cause the least performance hit. They are often hidden when performance tuning, and can create a real mystery and problem in that if you don't know they're there, but that's minimized if they are as fast as possible in everything they do.
Defaults would work for inserts. Definitely don't work for updates.
Personally, I really dislike this kind of audit trail for a number of reasons.
Yes, it says who and when on the last update, but it doesn't record what was done, just who did it and when. This means, for example, if I update column A to an incorrect value, and then you update column B to a correct value, the "audit" will show that you were the one who did the most recent update, and quite usually, you will get blamed for the wrong data in column A, even though you didn't touch that column. Even if you are able to prove you didn't touch column A, now NOBODY is responsible for it, because there's no record I ever touched that row, the moment the trigger puts your name/ID in the "UpdateBy" type column.
The same applies to data that was incorrect in the first place, if anything else has been corrected since. You have data for "CreatedBy", and "UpdatedBy", but don't have data on who did which. So, which one is responsible for changing an important client's last name to something offensive? You can't tell.
Some of this can be overcome if you can do point-in-time recovery, and restore to a state just before the last update. But that depends on the update having been done during the window you keep log backups for, and can end up requiring a LOT of extra work. Even if you have every backup ever taken for the database, going back five years, and you need to audit a row which was updated a dozen times in that span? You're going to have to restore the database a dozen times to get each update. That's a lot of work just to get an audit trail. And if you don't keep backups that are that old (very few businesses do), you simply can't audit the data worth a darn.
That's why I refer to it as a "false feeling of security audit trail" (or less polite terms, but not in a public forum). It makes people feel like they are auditing their data, when they really aren't. If this is "good enough", then really no audit at all is also "good enough", since unprovable audit data is worthless, or has negative value if it's used under the assumption that it's correct, when it really isn't.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
January 27, 2011 at 12:52 pm
Thanks for clarifying that Gail for I did have it partially wrong; good to know.
That said I just now have to hope that the explcit setting of these fields to NULL is a minimum. Logic woudl dictate that one would think why the heck would they explicitly set these to NULL to begin with? Unfortunately the would, I've watched it happen time and tike agin tracing the apps actions.
Thanks
Kindest Regards,
Just say No to Facebook!January 27, 2011 at 1:17 pm
GSquared (1/27/2011)
Like Gail, I'd be inclined to split the triggers up. SQL Server will want to have execution plans for them, and multiple internal branches generally result in expensive plans. You want triggers to finish as rapidly as possible so as to cause the least performance hit. They are often hidden when performance tuning, and can create a real mystery and problem in that if you don't know they're there, but that's minimized if they are as fast as possible in everything they do.Defaults would work for inserts. Definitely don't work for updates.
Personally, I really dislike this kind of audit trail for a number of reasons.
Yes, it says who and when on the last update, but it doesn't record what was done, just who did it and when. This means, for example, if I update column A to an incorrect value, and then you update column B to a correct value, the "audit" will show that you were the one who did the most recent update, and quite usually, you will get blamed for the wrong data in column A, even though you didn't touch that column. Even if you are able to prove you didn't touch column A, now NOBODY is responsible for it, because there's no record I ever touched that row, the moment the trigger puts your name/ID in the "UpdateBy" type column.
The same applies to data that was incorrect in the first place, if anything else has been corrected since. You have data for "CreatedBy", and "UpdatedBy", but don't have data on who did which. So, which one is responsible for changing an important client's last name to something offensive? You can't tell.
Some of this can be overcome if you can do point-in-time recovery, and restore to a state just before the last update. But that depends on the update having been done during the window you keep log backups for, and can end up requiring a LOT of extra work. Even if you have every backup ever taken for the database, going back five years, and you need to audit a row which was updated a dozen times in that span? You're going to have to restore the database a dozen times to get each update. That's a lot of work just to get an audit trail. And if you don't keep backups that are that old (very few businesses do), you simply can't audit the data worth a darn.
That's why I refer to it as a "false feeling of security audit trail" (or less polite terms, but not in a public forum). It makes people feel like they are auditing their data, when they really aren't. If this is "good enough", then really no audit at all is also "good enough", since unprovable audit data is worthless, or has negative value if it's used under the assumption that it's correct, when it really isn't.
For this particular table just knowing a row was updated and by whom is sufficient. We don't need to knwo what they changed so nuch as if a change did occur or not. We do have 15 minute incrmeentals so if a change did occur we have several weeks to go back and restore if need be. The main goal of this simple auditing is to verify if something did change and when it changed so we know where to go back to and restore from. With a 200+ GB DB its pain to keep doing test restores to find the right one.
Thanks guys
Kindest Regards,
Just say No to Facebook!January 27, 2011 at 1:21 pm
Yes, it's currently defined as adequate to know that a change was made. What happens if a row is changed five times in two minutes by four different people?
I'm not saying that's definitely an issue there. You know more about the data there than I do. It's just that every time I've seen this kind of audit being done, the people who designed it think it gives them things it really doesn't. Every time. Doesn't make it universal, just makes it every time I've seen it.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
Viewing 15 posts - 1 through 15 (of 15 total)
You must be logged in to reply to this topic. Login to reply