THE SQL Server Blog Spot on the Web

Welcome to SQLblog.com - The SQL Server blog spot on the web Sign in | Join | Help
in Search

Aaron Bertrand

Bad habits to kick : abusing triggers

In my last post in this series, I talked about choosing inappropriate data types.  This time, I want to touch on a few ways that I see triggers being misused. 


Using a trigger at all

A lot of people think that they need a trigger.  They allow direct access to tables instead of forcing data access through stored procedures, and then later realize that they want to control a modified date column or append rows to a DML audit table.  Since they have ad hoc queries in their application code, it is not practical to go out and add additional code to the existing queries.

Ideally, you should control data access via stored procedures, both for security reasons and so that you can control the DML that affects your tables.  In a stored procedure, you can apply conversions to incoming values, and supply data for unspecified columns, instead of having to deal with it in a trigger.  Already using stored procedures to update your table?  Great!  Just add the modified date column to your UPDATE statement, and you can disable your trigger. 

There will be cases where you won't have a choice but to use triggers.  You can't always convert to stored procedures, and you can't always prevent certain jerks people from bypassing your stored procedures and modifying the table directly.  Keep in mind that triggers can be disabled by people with sufficient permissions, so it is not your be-all and end-all as an auditing tool.


Not preparing for multi-row updates

A lot of people coming from an Oracle or other background assume that a trigger will fire for every row that is affected.  For SQL Server, this is not true; a trigger fires once per DML operation.  I often see code examples that look like this:

CREATE TRIGGER dbo.foo_update
ON dbo.foo
FOR UPDATE
AS
BEGIN
    SET NOCOUNT
ON;

    
DECLARE @bar INT;

    
SET @bar (SELECT bar FROM inserted);

    
-- other stuff
END
GO

Now, if you do something like:

INSERT dbo.foo(barSELECT UNION ALL SELECT 2;

You will receive a "subquery returned more than one value" error, and the original update will fail.  Oops!  You need to code your trigger to handle the inserted rows as a set, instead of expecting the inserted or deleted pseudo-tables to contain exactly one row.  There is no straightforward way to immediately convert your thinking to a set-based frame of mind.  I will say, if you find that the only way to solve this problem leads to a cursor in your trigger, stop what you're doing, build up a simple demonstration that explains what you are trying to do, and post your question on the SQL Server newsgroups, MSDN Forums or StackOverflow.

Note that there is potentially some confusing trigger behavior in SQL Server 2008 when using the new MERGE command; a trigger that handles multiple operations can fire multiple times.  Hugo Kornelis filed a bug on Connect; it was closed as "by design."

Performing inappropriate actions

A lot of times I've gone out of my way to convince people to stop performing quite expensive operations in their triggers.  The most common action I have seen is sending an e-mail.  I am not sure why people think it is a good idea to send e-mail from within a trigger; since the module is tied to the transaction that caused it to fire, now that transaction has to wait for the return of whatever process you're calling to dispatch the e-mail.  Similarly troublesome actions include functionality outside of SQL Server, such as COM / OLE automation objects (sp_OA*), extended procedures, and linked server queries.  Anything that takes you outside of SQL Server's direct and local control can become problematic quite quickly.

So what can you do to get around this?  Well you can certainly consider using Service Broker, which will allow you to perform any of these actions asynchronously; this allows your trigger to return control to the calling session immediately.  As an alternative to Service Broker, you could set up your own simple queue table, and have a background job that runs periodically and checks for any new tasks you've placed on this queue.  Again, this means your trigger only has to perform the insert into the queue table, and not force the calling transaction to wait for any subsequent actions that need to take place.


Summary

A trigger is not free, and can lead to various other potential problems in your application.  I have touched on a few of them here, but there are several others you will come across if you perform your due diligence.  Alex Kuznetsov went over several potential problems with triggers in great detail in his Defensive Database Programming series earlier this year.  Personally, I try to avoid them by solving problems in other ways; however, if you have to have them, it is important to understand some of the limitations and gotchas they can introduce to your environment.


I am working on a series of "Bad habits to kick" articles, in an effort to motivate people to drop some of the things that I hate to see when I inherit code.  Up next: using the visual designers in SSMS.


Published Monday, October 12, 2009 10:08 PM by AaronBertrand

Comment Notification

If you would like to receive an email when updates are made to this post, please register here

Subscribe to this post's comments using RSS

Comments

 

Helping people kick bad SQL Server habits « OTO One to One Interactive said:

October 12, 2009 10:11 PM
 

David (Dyfhid) Taylor said:

Aaron,

Forgive me Master, I am but learning, and in my learning, I thought I was being brilliant.

I have a web page that actually runs on a file share, with a form. (They took away my server, I had to do something!) The page has VBScript on it, that collects form data and Inserts it into table on our SQL Server. Things need to be done with that data, and immediately!

The table has a trigger.

I thought I was doing so well! This thing not only writes an email, it goes out to the filesystem to find any files uploaded to the form (handled on the web page script) to attach them. Then, after the send_dbmail, (blind copying me, so I know it ran - I said brilliant, right?) it deletes the file (no error checking, you can always delete a file, right? Yeah.)

After doing whatever it does there (*usually* good things), it then EXECs a SP that again goes out to the filesystem, and, through OLE Automation, (sp_OA*, do you mention that above? I *must* be doing things right!) opens not one, but two spreadsheets, inserts information to them, saves them, and finally returns control to the webpage, which puts up a message box that life is good, reassuring the poor user who has been sitting there staring at a web page for several seconds after clicking Submit.

Is this bad? Is this what you mean by trigger abuse? But I meant no harm! I thought I was doing what was right! I thought... never mind what I thought, I am a self taught Accidental DBA/Developer, my thinking is skewed. I'll go fix it. Thanks for pointing out the error of my ways.

Dyfhid

October 12, 2009 11:38 PM
 

Brian Tkatch said:

I think it's a bad habit to use TRIGGERs at all. Better to lock down the TABLE and only GRANT EXECUTE on PROCEDUREs that define all actions that need to be taken.

I use TRIGGERs for logon security, db maintenance (where required), and things that just are not part of the application, per se.

October 13, 2009 9:33 AM
 

AaronBertrand said:

Brian, I agree completely.  However, in some cases people don't have the ability to make such sweeping changes overnight, because existing applications need to continue to function.

October 13, 2009 9:39 AM
 

Brian Tkatch said:

>Brian, I agree completely.

Excellent. I was hoping that's what you meant. Between that and the CHAR/VARCHAR comments about space savings, i am very happy you posted these. Perhaps a summary at the end with one line (and a link) for each? A quick sheet, that's all.

>However, in some cases people don't have the ability to make such

>sweeping changes overnight, because existing applications need to

>continue to function.

Aah, instead of Aaron Bertand thou shall be knownst as Aaron the Benevolent.

October 13, 2009 11:44 AM
 

Bruce W Cassidy said:

"Ideally, you should control data access via stored procedures, both for security reasons and so that you can control the DML that affects your tables."

Those are good things, absolutely.  But they miss the value of using something like stored procedures to access the database.

You can use stored procedures to create an "API".  This helps reduce the intimate coupling between an application and the database.  With an API in place, if you need to redesign how some data is stored, you can often do so without having to redesign the API.

Reducing coupling is a software design principle.  It's fundamental to well designed software.  But for some reason, people think it's okay for applications to poke around directly in a database.

Just like any other form of code, database code is best when it is well designed.  And the interface between an application and the database is important.  A well-designed API is critical to the longevity of an application.

So when people ask me why do I not recommend triggers, my point is usually around the need to break a design fundamental.  You don't go poking around directly in tables any more than you go calling code buried in the middle of a module.  You use the API.  And with a good API, there is almost never a need for triggers.

October 18, 2009 3:56 PM
 

Thomas said:

Over my many years of development, I have discovered that stored procedures are not always sufficient to protect data integrity. Sometimes, you have to bake the rules into a trigger to prevent the amateur DBA from screwing the data. Typically, this amateur DBA has been with the company for some time, or in egregious cases, is the CEO/Owner themselves. Using triggers to enforce unusual and important data integrity rules is about the only place where I use triggers.

October 18, 2009 4:27 PM
 

AaronBertrand said:

I guess what I was stating when I talked about controlling the DML that affects your tables is that you should be forcing your applications and users to use the API (the stored procedures).

I agree with Thomas, that there are some cases where you can't *force* the use of the API, you can only "encourage."  My point at the end of the article is that, in those cases where you have to have them, do your homework.

October 18, 2009 4:43 PM
 

ivanrdgz said:

I don't like either sending emails from inside the trigger, but recently when I mentioned this to a colleague he replied that the sp_send_dbmal just post the message into the queue. I still don't like this approach but what do you have to say about this?

October 20, 2009 5:52 PM
 

Aaron Bertrand said:

I still don't like it, for a few reasons.

1. if you should ever switch from the built-in mail solution to something else (I had to back in the early 2005 days), you will have to ensure it uses a similar methodology, or potentially re-write your triggers.  You'll have to change all your triggers regardless, because you've hard-coded the delivery mechanism in each one!

2. using your own queue table, you can quickly and easily adjust the frequency of e-mails relative to the number of trigger invocations.  Imagine a row is updated 500 times, you're going to send 500 e-mails, when perhaps only the last one is relevant.  With a queue table you can easily summarize all the new rows since the last poll, and send a single e-mail with either counts or each line item depending on the requirements.

A very low cost up front design can provide both flexibility and scalability without losing a single thing.  

October 20, 2009 6:05 PM
 

Tom Thomson said:

Inapprpiate actions is not just an issue with triggers; as a general rule these actions should never be performed when any locks are held - never performed within a transaction.  Dumping these actions off onto an asynchronous queue handler is not much help if that queue handler holds an exclusive lock on the queue while performing that action (I've seen this done: begin transaction, take the date for 1 email fromk the queue, delete the data, construct the email, send it, if send failed roll back else commit: criminally stupid, in my opinion).SO performing inappropriate actions within a trigger is just another instance of performing inapproopriate actions within a transaction and shouldn't be regarded as something affecting triggers only.

October 22, 2009 11:14 AM
 

AaronBertrand said:

Tom, I agree.  However I treated the case in triggers because:

(a) treating bad habits people have wrt transactions is a much lengthier topic.  This had enough weight with triggers alone to form a decent post and generated some good dialog as well.

(b) I find that the "sending an e-mail from a trigger" requirement is extremely more common than the "sending an e-mail from within a transaction" requirement.  I don't doubt that the latter exists, in fact I am sure it is quite rampant.  But when I have my community support hat on, I see all too often the request about how to send e-mail from a trigger, or why is my trigger so slow because I'm sending an e-mail, or how can I send 30 e-mails from my trigger without using a cursor?

October 22, 2009 11:19 AM
 

HardCode said:

Aaron, how would you weigh triggers to enter records into an audit table of the target table? We use triggers for this, because we have to capture all changes made to all records for regulatory reasons.

January 11, 2010 5:10 PM
 

AaronBertrand said:

HardCode, like I said:

"There will be cases where you won't have a choice but to use triggers."

In the case you are talking about, I think it is still possible to avoid triggers (e.g. by denying all write access to the base tables and forcing modifications through stored procedures), but the conversion may take longer than the time the enforcement needs to start happening, and aside from that, may not be worth it.

My comments in the first section are not meant to imply, "nobody should ever use triggers," more along the lines of, "a lot of people use triggers when they don't need to."

January 11, 2010 5:17 PM
 

John Smith said:

I started reading with interest, until I got to the word "jerks" crossed out. You called for this: jerk you! Sproc-only data access is your jerking opinion, and it's obviously not the only correct opinion since there are a ton of people not sharing it with you (otherwise, non-sproc access wouldn't even exist). So, before you call people names, next time use your jerking old-fashioned dba brain to think for a second. Jerk!

January 25, 2010 4:42 PM
 

Aaron Bertrand said:

Wow, thin skin, Mr. Anonymous?

The jerks i was referring to, if you had bothered to read the context, are the ones who work in an environment where there is a procedure-only policy in place, they just choose to knowingly circumvent the policy because they can, don't care, are lazy, what have you.

I'll say it another way to make it clear: I was not in any way calling people who don't use stored procedures jerks; I was talking about the people who don't find it useful to follow existing conventions in a particular environment.

I'm sorry the concept was lost on you and that you misinterpreted my intent.  If that makes you call me names and find less value in my content, so be it.  But if you ever work for me, be prepared to stand down.

I may have pretty strong opinions about things like this, but that's the beauty of a blog: I'm allowed to state my opinions.  At least I put my name behind mine.  Perhaps I should disable anonymous comments to bypass having to defend my opinions from drive-by shooters like yourself.

Cheers,

Aaron

January 25, 2010 6:17 PM

Leave a Comment

(required) 
(optional)
(required) 
Submit

About AaronBertrand

...about me...

This Blog

Syndication

Powered by Community Server (Commercial Edition), by Telligent Systems
  Privacy Statement