Since Triggers are a contentious subject, can someone please tell me if mine is written correctly?

Question

Since Triggers are a contentious subject, can someone please tell me if mine is written correctly?

isuckatsql

SSChampion

Points: 10381
More actions
May 12, 2014 at 7:39 pm

#307930

/****** Object: Table [dbo].[TriggerCB] Script Date: 5/12/2014 9:20:12 PM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
SET ANSI_PADDING ON
GO
CREATE TABLE [dbo].[TriggerCB](
[IDa] [int] IDENTITY(1,1) NOT NULL,
[LocationState] [varchar](2) NULL,
[StateCount] [int] NULL,
CONSTRAINT [PK__CBtriggerid] PRIMARY KEY CLUSTERED
(
[IDa] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO
SET ANSI_PADDING OFF
GO
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
SET ANSI_PADDING ON
GO
CREATE TABLE [dbo].[TriggerCBCount](
[IDb] [int] IDENTITY(1,1) NOT NULL,
[StateCountb] [int] NULL,
[LocationState2] [varchar](2) NULL,
CONSTRAINT [PK__CBtriggerid2] PRIMARY KEY CLUSTERED
(
[IDb] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO
SET ANSI_PADDING OFF
GO
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TRIGGER [dbo].[StateCountInsert2x] ON [dbo].[TriggerCB] FOR INSERT AS
BEGIN
UPDATE u
SET u.StateCountb = u.StateCountb + d.cnt
FROM [dbo].[TriggerCBCount] u
inner join
(
SELECT LocationState, count(*) as cnt
FROM inserted
GROUP BY LocationState
) d
--ON u.IDb = d.IDa
ON u.LocationState2 = d.LocationState
END
GO
The trigger works fine, i just want to make sure it is written correctly.
I am doing the data import via an SSIS project.
Thanks

Viewing 10 posts - 1 through 9 (of 9 total)

You must be logged in to reply to this topic. Login to reply

Sean Lange SSC Guru Points: 286590 More actions · Answer 1

I don't see anything wrong with the code. I would however point out that what you are doing is very brittle. You are storing aggregate data from one table in another table. This type of calculated values can, and most likely will, get out of synch at some point. If you really want to continue you need to create more than 1 trigger. You will also need update and delete triggers if you want to have any chance at keeping that data accurate.

I would consider using a calculated column on TriggerCBCount. That way it is always up to date and you don't have to mess around with triggers.

_______________________________________________________________

Need help? Help us help you.

Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.

Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.

Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/

Jack Corbett SSC Guru Points: 184394 More actions · Answer 2

Sean Lange (5/13/2014)
I don't see anything wrong with the code. I would however point out that what you are doing is very brittle. You are storing aggregate data from one table in another table. This type of calculated values can, and most likely will, get out of synch at some point. If you really want to continue you need to create more than 1 trigger. You will also need update and delete triggers if you want to have any chance at keeping that data accurate.
I would consider using a calculated column on TriggerCBCount. That way it is always up to date and you don't have to mess around with triggers.

Agree with Sean 100%.

Jack Corbett
Consultant - Straight Path Solutions
Check out these links on how to get faster and more accurate answers:
Forum Etiquette: How to post data/code on a forum to get the best help
Need an Answer? Actually, No ... You Need a Question

Michael L John One Orange Chip Points: 27060 More actions · Answer 3

A calculated column is likely the best solution here.

Michael L John
If you assassinate a DBA, would you pull a trigger?
To properly post on a forum:
http://www.sqlservercentral.com/articles/61537/

isuckatsql SSChampion Points: 10381 More actions · Answer 4

I have done some digging around on the forums but can't find an example that would work.

Could you give me an idea on how to do this?

Thanks

Sean Lange SSC Guru Points: 286590 More actions · Answer 5

isuckatsql (5/13/2014)
I have done some digging around on the forums but can't find an example that would work.
Could you give me an idea on how to do this?
Thanks

To do this you would have to use a UDF to calculate the value. This is a potential performance nightmare though. Because it is using a UDF the values can't be persisted which means that each and every time you query the table it will have to execute the scalar UDF for each row returned.

Maybe you could create a view instead so you can easily calculate the values as you need them?

_______________________________________________________________

Need help? Help us help you.

Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.

Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.

Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/

isuckatsql SSChampion Points: 10381 More actions · Answer 6

lol ..... and so we go full circle.

I started with a dynamic SQL query, too slow.

Then i went with an SQL Indexed View, much faster, but the more items i added a count on, the slower the query became.

Then i tried a SQL Trigger, which is working fast, but i have not done any load testing for true performance.

Then a SQL computed column was suggested, and now rejected, and now we are back at the SQL View 🙂

Thanks for your help, i will keep trying the SQL Trigger vs SQL indexed View.

..... unless a better suggestion comes along.

Sean Lange SSC Guru Points: 286590 More actions · Answer 7

isuckatsql (5/13/2014)
lol ..... and so we go full circle.
I started with a dynamic SQL query, too slow.
Then i went with an SQL Indexed View, much faster, but the more items i added a count on, the slower the query became.
Then i tried a SQL Trigger, which is working fast, but i have not done any load testing for true performance.
Then a SQL computed column was suggested, and now rejected, and now we are back at the SQL View 🙂
Thanks for your help, i will keep trying the SQL Trigger vs SQL indexed View.
..... unless a better suggestion comes along.

I don't see how a view would be very slow.

SELECT LocationState, count(*) as cnt

FROM TriggerCB

GROUP BY LocationState

That should be pretty darn fast.

_______________________________________________________________

Need help? Help us help you.

Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.

Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.

Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/

isuckatsql SSChampion Points: 10381 More actions · Answer 8

Sean Lange (5/13/2014)
isuckatsql (5/13/2014)
lol ..... and so we go full circle.
I started with a dynamic SQL query, too slow.
Then i went with an SQL Indexed View, much faster, but the more items i added a count on, the slower the query became.
Then i tried a SQL Trigger, which is working fast, but i have not done any load testing for true performance.
Then a SQL computed column was suggested, and now rejected, and now we are back at the SQL View 🙂
Thanks for your help, i will keep trying the SQL Trigger vs SQL indexed View.
..... unless a better suggestion comes along.
I don't see how a view would be very slow.
SELECT LocationState, count(*) as cnt
FROM TriggerCB
GROUP BY LocationState
That should be pretty darn fast.

The SQL Indexed View queries take approx 0 ms on about one million records, which is great!

The dynamic SQL query that creates the page data, takes approx 0.1 seconds, which is great!

At the moment i have five SQL Indexed View Queries, each taking approx 0 ms on initial page load, and the dynamic SQL code taking 0.1 second, which is a total of approx 0.10 seconds to load the initial page with data.

When i search for a subcategory in the count column that has a lot of counted records eg 500k, the page performance suddenly goes from 0.1 seconds to 0.9 seconds, which is too slow!

When i do the same subcategory search using just the dynamic query it still only takes approx 0.1 seconds, so i am trying to find the loss of 0.8 seconds and fix the issue.

Thanks

Jeff Moden SSC Guru Points: 1004415 More actions · Answer 9

The problem with indexed views isn't during SELECT's. The problem with indexed views is with inserts to the underlying tables and the bigger the data gets, the longer the inserts will take because of the recalculations that the aggregates need to do.

The use of the trigger that you've written is pretty much like I suggested on the other thread... which I've lost track of.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)