trigger always fires.. 1

[warkre]

I have a trigger that shout fire when 1 field in the record is updated. So I dug up this for a test:

USE almanak;
GO
IF EXISTS (SELECT name FROM sys.objects
      WHERE name = 'actie_uitgevoerd' AND type = 'TR')
   DROP TRIGGER actie_uitgevoerd;
GO
CREATE TRIGGER [dbo].[actie_uitgevoerd]
ON [almanak].[dbo].[almanak_acties]
AFTER UPDATE
AS

IF ( UPDATE (a_uitgevoerd) )
BEGIN 
	RAISERROR ('actie is klaar', 16, 10)
END;
GO

As I understood this should not *ALWAYS* trigger on an update.
Can someone shed some light on this?

 

[HarleyQuinn]

What is it you want to know?

HarleyQuinn
---------------------------------
The most overlooked advantage to owning a computer is that if they foul up there's no law against wacking them around a little. - Joe Martin

Get the most out of Tek-Tips, read FAQ222-2244 before posting.

 

[warkre]

Sorry, I wasn't clear enough. Been busy with it too long already.
I want the trigger only to fire when a_uitgevoerd has changed from False to True so I can fill in the date/time in another field in the same record.
a_uitgevoerd is a bit field, the date/time field is a varchar. There is a primary key field, a_actie.

But as I am trying different things, it seems as if the application reads the whole record and then writes everyting back. So I think all fields seem to have changed.
(the Raiserror prints out ALL fields of the record with an OK)
So should I do someting with the deleted and inserted tables, but I do see how to do that.

 

[warkre]

I have this code so far. It works although the dateformat has to be made dd-mmm-yy hh:mm:ss

USE almanak;
GO
IF EXISTS (SELECT name FROM sys.objects
      WHERE name = 'actie_uitgevoerd' AND type = 'TR')
   DROP TRIGGER actie_uitgevoerd;
GO
CREATE TRIGGER [dbo].[actie_uitgevoerd]
ON [almanak].[dbo].[almanak_acties]
AFTER UPDATE 
AS
BEGIN

IF @@ROWCOUNT =0
RETURN

DECLARE @actie int
IF UPDATE(a_uitgevoerd)
BEGIN
	SELECT @actie = i.a_actie
	FROM inserted i JOIN deleted d
	ON i.a_actie = d.a_actie
		AND COALESCE(i.a_uitgevoerd,-1) <> COALESCE(d.a_uitgevoerd,-1)
		AND i.a_uitgevoerd = 1
	UPDATE [almanak].[dbo].[almanak_acties]
	SET almanak_acties.a_af_datum = GETDATE()
	WHERE @actie = almanak_acties.a_actie
END
END

 

[ESquared]

That trigger will only work for single-row updates. Please don't use it.

 

[SQLDenis]

IF ( UPDATE (a_uitgevoerd) )

will always fire even if the values don't change

read Fun With SQL Server Update Triggers

Denis The SQL Menace
--------------------
SQL Server Code,Tips and Tricks, Performance Tuning
SQLBlog.com, Google Interview Questions

 

[ESquared]

IF ( UPDATE (a_uitgevoerd) )

will always fire even if the values don't change

I know what you're trying to say but the way you said it is ambiguous, because the value DID get updated: to the same value.

IF UPDATE() will always be true if the column is included in the update statement, even if the updated value is the same as the original value.

However, IF UPDATE() will return false if the column is NOT included in the update statement, and thus we know for certain that the value of that column did NOT change.

Also Denis, you have to be very careful with uses like your "COALESCE(i.value,-1) <> COALESCE(d.value,-1)" statement:

• You have to know in advance that -1 will NEVER be possible in the table, so you'd best enforce this with a check constraint.

• If the column is a character data type and its collation is not case sensitive or not accent sensitive, then you need to use a case-sensitive/accent sensitive collation in your <> comparison so strings that differ only in capitalization or accents do not falsely compare as equal (which could have effects like preventing the user from making the update just to correct case or alter accent marks, a legitimate update scenario). It might even be best to use a binary collation that will always respect any changes, no matter what collation is being used.

 

[SQLSister]

Learn to write joins to inserted and deleted in your update statment. Look in BOL for the syntax.

Now for some further education. Your existing trigger does not work. You may think it works but it doesn't because it only handles a single record update. If multiple records were updated (and you must always assume this will happen when writing a trigger) your trigger would fail miserably.
Run this code and you will see why

create table #temp (test int, testdate datetime)

insert into #temp
select 1, getdate()
union
select 2, getdate()
union 
select 3, getdate()

declare @test int
select @test = test from #temp
select @test

Do you see how the value of the variable is only reflects the value of the last record? Doing this in a trgger when you have a multi record update means that only 1 record will be updated by the following update statement, rather than all the records which meet the criteria. Even worse, it won't fail and you won't know it is not working until you have a thoroughly messed up (and probably unfixable)database.

Do not ever write a trigger assigning a value to a variable by joining to either inserted or deleted. This is a data integrity issue and will cause problems in your database sooner or later. You not only must immediately rewrite this to process multiple record updates, but you need to review every single trigger in your database to make sure none of them do the same thing. If you do not, you will have data integrity problems that will be almost impossible to fix.

Some pseudocode for you to help you figure out what you should be doing instead of what you did.

update table1
set mydatefield = getdate()
from table1 t
join inserted i on t.idfield = i.idfield
join deleted d on t.idfield = d.idfield
where i.somefield <> d.somefield

I can't emphasize too strongly how very bad the technique you used is. This is the kind of thing people lose their jobs over when the failure comes to light.

"NOTHING is more important in a database than integrity." ESquared

 

[warkre]

Thanks for pointing out the problem I created.
I will fix it so it will work on multiple updates.
The thing is in my environment the database is only accessed through an application that can only update one record. On the left part of the screen there is a listbox with items and on the right side there is one selected record that you can modify. And nobody, beside myself, has direct access to the database.
But to learn to program properly I should do it right, right?

 

[ESquared]

It is more than that. In theory, there is no difference between theory and practice. But in practice, the difference can be enormous.

Eventually, you or someone else will make direct data changes in your database, outside the application. When that happens, the database should have been designed so that total data mangling due to an otherwise innocent insert or update is simply not possible. You also cannot absolutely guarantee that the application will always, in all cases and forever, do its updates one row at a time. What if a new developer comes after you are gone?

An analogy might be, you're building a car that only you drive, and there is a button that ejects the driver, with possibly fatal results. YOU know not to push the button, so the car is fine, right? But what happens to the next guy who comes along? Or to you the day you forget? Best to design the car so disaster isn't possible at all.

This consideration is much more than a theoretical sort of learning to program properly.

 

[warkre]

I know from practice that practice <=/?> theory.
Thus this is what I should (and am) use(ing):

USE [almanak]
GO
/****** Object:  Trigger [dbo].[actie_uitgevoerd]    Script Date: 04/04/2008 10:54:20 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER TRIGGER [dbo].[actie_uitgevoerd]
ON [almanak].[dbo].[almanak_acties]
AFTER UPDATE 
AS
BEGIN

IF @@ROWCOUNT =0
RETURN

IF UPDATE(a_uitgevoerd)
BEGIN
	update [almanak].[dbo].[almanak_acties]
	set a_af_datum = i.a_laatste_mod_dt, a_door_gebruiker = i.a_laatste_mod_wie
	from [almanak].[dbo].[almanak_acties] a
		join inserted i on a.a_actie = i.a_actie
		join deleted d on a.a_actie = d.a_actie
		where COALESCE(i.a_uitgevoerd,-1) <> COALESCE(d.a_uitgevoerd,-1)
        		AND i.a_uitgevoerd = 1

END
END

and BTW: a_uitgevoerd is a (bit,null) value, so can (in theory) not be negative in a binary system.
It is a flag that says the action is completed.
The routine then puts the username and time of the last modification in the fields for completion.

Am I now allowed to take a short holyday?
I promise to never do this again this way.
-Warner-

 

[ESquared]

Hurrah! You did it.

I hope my post didn't seem harsh, I was just trying to be compelling. A star for you...