Writing a trigger on a tbl on insert, to insert data into another tbl

[bind]

Hello all,

I'm having problems creating a trigger for the following task,

I have a table called dbo.users, I'm going to be creating a trigger on this table and in the trigger I need it to grab the newly inserted "UserID" field and put it into a variable called @UserID.

Then I have to do a select statement to select all of these FormID's from another table called dbo.form_tbl. It may return 10 or so records, maybe more. then I need to put those returned FormID's into a variable named @FormID and for EACH row returned, need to do a insert into a table named dbo.StatusUsers where the values will be (@UserID, @FormID, '0','0')

But it needs to do this for each record returned from the above select statement in dbo.form_tbl.

I'm having a heck of a time writing the actual trigger, any help on this would be greatly apprecieated.

Thanks all

 

[gmmastros]

and put it into a variable called @UserID.

then I need to put those returned FormID's into a variable named @FormID and for EACH row returned...

If you follow this logic, you are in for a world of pain. Let me explain...

Triggers can be the trickiest code to write in a database. the problem with triggers is that they will "fire" when one OR multiple rows are inserted/deleted/updated. When you make statements like "put in variable", it's a clear indication that you are NOT considering the situation where multiple rows are affected simultaneously.

The problem is.... suppose you do have a situation where multiple users (@UserId) are added simultaneously (think of a data import for example). If you continue with the variable approach, depending on how you write your code, you will either get an error or get just the first one. Either way is bad, but getting just the first one is worse because there would be no indication that there are missing rows in the StatusUsers table.

Then, you make another statement "for EACH" which leads me to think you are considering looping code (while loop or cursor). Both of these are bad in triggers because SQL server is horrendously slow using loops.

The following code is untested, but will hopefully point you in the right direction.

Insert 
Into   StatusUsers(UserId, FormId, Column3, Column4)
Select I.UserId, form_tbl.FormId, '0', '0'
From   Inserted I
       Cross Join form_tbl

I strongly encourage you to test the trigger in a 1000 different ways. For the sake of your data integrity, it's extremely important that you get it right.

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[markros]

Put this code in the trigger

;with cte_JustInserted as (select UserID from Inserted)

insert into StatusUsers (UserID, FormID, Field3, Field4) select F.FormID, C.UserID, 0, 0 from dbo.Form_tbl F inner join cte_JustInserted C on F.UserID = C.UserID

 

[bind]

gmmastros,

Thank you for the help and for the information, I see the flaws in my original logic. Forgive my ignorance, still a little 'wet behind the ears' in SQL

I was able to get your example working with a little modification

Insert Into dbo.StatusUsers(UserId, FormId, Finished, Hide)
Select I.user_id, dbo.form_tbl.form_id, '0', '0'
From dbo.users I
Cross Join dbo.form_tbl

Worked perfect, thats exactly what I needed.

I guess I was looking at the problem like I would working with loops and data in vb.net, it's a whole different world in SQL.

Anyhow, thanks again

 

[bind]

markros,

Thanks for the example, apprecieate the help.

 

[markros]

Actually, George may be right with cross join. I assumed you have UserID field in your forms table. If you don't, then you may want to use CROSS JOIN instead of the inner join.

Also I think we don't really need a cte for Inserted, we can use Inserted table directly in the code - don't know why I wanted to put it here - may be out of my love for CTE.

 

[markros]

Bind,

Our posts crossed in time. Also, when I first replied I didn't see George's post.

 

[gmmastros]

Forgive my ignorance, still a little 'wet behind the ears' in SQL

We were all there at one time. Don't worry about it.

In SQL, there are very few occasions where you NEED to use a loop. As a SQL beginner, I strongly encourage you to NOT use them. If you come across a situation where you think you need to use a loop, I want you to post a question here. 99% of the time, there is a set based method that will accomplish the same results, will run faster, and (ultimately) will be easier to understand.

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[bind]

Markros,

Gotcha, no worries.

One more question for either of you,

Is there any way to wrap that insert in a "IF NOT EXISTS"?

Just so I'm not always inserting the same data over and over

 

[gmmastros]

IF NOT EXISTS may not be the right choice of functionality here. It appears as though you will be inserting multiple rows in to the StatusUsers table. If Exists would be a good choice if you were looping.

Instead, I recommend a left join to the statusUsers table with an IsNull check. Like this:

Insert Into dbo.StatusUsers(UserId, FormId, Finished, Hide)
Select I.user_id, dbo.form_tbl.form_id, '0', '0'
From   Inserted I
       Cross Join dbo.form_tbl
       Left Join dbo.StatusUsers
         On I.UserId = StatusUsers.UserId
         And form_tbl.Form_Id = StatusUsers.FormId
Where  StatusUsers.UserId Is NULL

Again, test this 1000 different ways before you release this code to your production database.



-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[bind]

Gmmastros,

Thanks again, it's funny, you almost have to develop an abstract way of thinking when you're working with SQL. I can tell that portion is going to take a while to grasp hehe.

 

[gmmastros]

You're right. But.... it's extremely important that you "get it" because it can have a dramatic impact on performance. In my opinion, when dealing with a database, it's extremely important to make sure your data is accurate. A close second is performance. You can write all your queries involving loops to get the data right, but the performance will be horrible. Getting the data right, and doing it quickly is the goal.

I encourage you to do a little research on [google]thinking in sets[/google]. You'll be glad you did.

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[bind]

Awesome, thank you so much for the information, I'll be sure to check that link out. I really apprecieate all of the help and wisdom.

 

[SQLSister]

George, I'm laughing becasue I've been known to say data integrity is first, performance and security are a close second. Very close to what you wrote. I get very annoyed with people who think that they should never consider performance in design because it's "Premature optimization". Ot that a few minutes extra of dev time is more important than user time (no problem that it takes the users an extra hour a day every day, we saved ten minutes of expensive dev time with this known poorly performing technique). Ugh, no wonder there are so many really lousy performing databases out there.

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

 

[gmmastros]

That's nothing....

I spent 1/2 my morning modifying a single stored procedure (that I wrote 2 years ago). I improved the performance from 1.5 seconds to 13 milliseconds.

In my opinion, performance is extremely important for several reasons. First, nobody likes to run an application that is slow. Second, you never know when your user or database size will increase. A "fast enough" procedure with 1 user and 20 rows in your table may not be fast enough when there is 100 users and 20 million rows in your table.

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom