November 19, 2013 at 12:40 pm
In an Insert Trigger this code is sometimes quite slow: "SELECT @numrows = @@rowcount"
This code is used to determine if a row was inserted into the table. So 2 questions.
1. Isn't this redundant? How can you get to an insert trigger if a row isn't being inserted?
2. Sometimes the above line of code is has a very long duration (like 37450ms in a sql trace). Any clues to point me in a direction?
My initial thought is to just delete the code. I am working with a commercial package so I have to be a little careful.
Thanks for reading!
John
November 19, 2013 at 12:45 pm
John Hanrahan (11/19/2013)
In an Insert Trigger this code is sometimes quite slow: "SELECT @numrows = @@rowcount"This code is used to determine if a row was inserted into the table. So 2 questions.
1. Isn't this redundant? How can you get to an insert trigger if a row isn't being inserted?
2. Sometimes the above line of code is has a very long duration (like 37450ms in a sql trace). Any clues to point me in a direction?
My initial thought is to just delete the code. I am working with a commercial package so I have to be a little careful.
Thanks for reading!
John
Without more details about the trigger it is hard to say for sure. Are there multiple statements in this triggers each of which references @numrows? If so then you might need it. I have a bad feeling that this does some sort of looping or something, at least my experience suggests that as a high probability. When you see the trigger worrying about @@rowcount it certainly raises a red flag that you should investigate that trigger.
Can you post the code for the trigger? We may want/need more details but that is the very least amount of information we would need to get started.
_______________________________________________________________
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/
November 19, 2013 at 12:48 pm
Sean,
Here's the relevant code:
ALTER TRIGGER [dbo].[tI_timCustItem] ON [dbo].[timCustItem] FOR INSERT AS
/* Copyright (c) 1995-2011 Sage Software, Inc. All rights reserved. */
BEGIN
DECLARE @numrows INT,
@nullcnt INT,
@validcnt INT,
@errno INT,
@errmsg VARCHAR(255)
SELECT @numrows = @@ROWCOUNT
IF @numrows = 0
RETURN
SET NOCOUNT ON
/* tciUnitMeasure R_2279 timCustItem ON CHILD INSERT RESTRICT */
IF UPDATE(LastPriceUOMKey)
BEGIN
SELECT @validcnt = COUNT(*)
FROM inserted, tciUnitMeasure WITH (NOLOCK)
WHERE
inserted.LastPriceUOMKey = tciUnitMeasure.UnitMeasKey
SELECT @nullcnt = COUNT(*) FROM inserted WHERE
inserted.LastPriceUOMKey IS NULL
IF @validcnt + @nullcnt <> @numrows
BEGIN
SELECT @errno = 50002,
@errmsg = 'Unable to add the timCustItem record because it is trying to reference a record in tciUnitMeasure that does not exist. Reference ID: (R_2279).'
GOTO error
END
END
Maybe my question should be isn't @@rowcount always one 1 in a trigger? You can see they use @numrows to do error checking? I guess it depends on the answer for @@rowcount.
Thx.
November 19, 2013 at 12:55 pm
John Hanrahan (11/19/2013)
Sean,Here's the relevant code:
...
Maybe my question should be isn't @@rowcount always one 1 in a trigger? You can see they use @numrows to do error checking? I guess it depends on the answer for @@rowcount.
Thx.
Absolutely not. In SQL server triggers DO NOT fire row by row. They respond to a statement, which in this case is an insert.
I don't what datatypes of whatever are in this table but consider the following insert statement.
insert timCustItem (LastPriceUOMKey)
select 10 union all
select 20
Assuming that statement would be valid the inserted table would have 2 rows.
_______________________________________________________________
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/
November 19, 2013 at 1:00 pm
Question: a select count(1) from inserted ill return the number of inserted rows?
just for the record: avoid triggers, they are evil.
avoid to put business logic in the database, let the application to handle it in the appropriate layer.
November 19, 2013 at 1:11 pm
jcb (11/19/2013)
Question: a select count(1) from inserted ill return the number of inserted rows?just for the record: avoid triggers, they are evil.
avoid to put business logic in the database, let the application to handle it in the appropriate layer.
I agree about triggers but as the OP stated this is a 3rd party app.
I am working with a commercial package so I have to be a little careful.
To be honest if I were going to use a trigger for this type of thing...which I would most certainly not I would totally rewrite this things.
This entire trigger looks to me to be a replacement for a foreign key. Basically the logic here is to not allow a value of LastPriceUOMKey that doesn't already exist in tciUnitMeasure. And don't get me started on NOLOCK...
_______________________________________________________________
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/
November 19, 2013 at 1:25 pm
But any idea why @@rowcount would be so slow? The line of code doing the insert is a single row insert. I can't understand why it would be so slow.
As for the foreign key constraints. The product was first written for SQL 6.0 and they only allowed 16 levels of cascading deletes (as I recall), maybe it was even 8 back then. They had to use triggers instead of RI. They never upgraded their code or design. In fact the company that writes the main product has decided to kill the product we are using.
November 20, 2013 at 5:57 am
Sean, you are right! Sorry John I missed the fact its a black box.
Sean you are right, again, about the FKs. Maybe the OP can just disable/drop that trigger and create a FK but that can impact the application error handling.
John, are you using MS SQL 6 or just a 2008 with compatibilities options?
How do you get that specific line inside the trigger is the problem?
Can you create a simulation environment just to test it?
Had you tried to change that line to
SELECT @numrows = select count(1) from inserted
or just
SET @numrows = 1
and see if it changes performance?
That performance issue smells as something is waiting other things to complete (did you got any deadlock or race condition?)
Is that insert part of a big transaction?
November 20, 2013 at 6:15 am
jcb (11/19/2013)
just for the record: avoid triggers, they are evil.
They are a tool. They can be used well or used badly. When used badly, blame the developer, not the tool.
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
November 20, 2013 at 6:18 am
John Hanrahan (11/19/2013)
But any idea why @@rowcount would be so slow? The line of code doing the insert is a single row insert. I can't understand why it would be so slow.
It may not be that statement that's slow.
If you can change the trigger, add this line before that SELECT and then see what statement gets the high execution
SELECT @errmsg = NULL
It does nothing, I just want to see if the long delay is in the firing of the trigger or the exact SELECT itself.
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
November 20, 2013 at 9:04 am
Gail,
I can't do that (change the code). That could materially change how the trigger works. While in this case it is a single insert (via a values ()), I'm not sure about the others. I do suspect this is not the problem (@@rowcount) but thought maybe someone had run into this before.
As for SQL 6.0. They system now runs up to SQL 2012 but the vendor has not upgrade their database design or much of their code to take into account new standards and features. I have found this to be very common with the accounting packages I have worked with (that they don't upgrade working code).
John
November 21, 2013 at 6:15 am
GilaMonster (11/20/2013)
jcb (11/19/2013)
just for the record: avoid triggers, they are evil.They are a tool. They can be used well or used badly. When used badly, blame the developer, not the tool.
Gail, you are right (as usual).
But triggers, cursors, denormalization and other DB tools are misused most of time by developers.
Cannot remember last time I found a trigger is not wrong used to implement a business logic better to be left in the application.
As a developer I try hard to put that tools to good use, only.
John, if you must stick with that application as it was made you are right to take extra caution before changing anything.
I also suspect that line is not the problem.
Make some profiling and keep a eye in that inserts query plan.
A spare server and a DB backup can be great to explore and test some solutions.
I hope you find the culprit and fix it (and share it at the forum).
November 21, 2013 at 6:25 am
ignore...
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
November 21, 2013 at 8:21 am
Well I'm not 100% sure but I think we've found the problem. The system we use is interconnected with several other systems some of which are a bit old. It looks like the Windows Heap was somehow causing issues, technically the software that uses it. We solved it by running a Microsoft Heap analyzer product which saw it getting full. I don't understand how that could effect SQL but somehow it looks like it was. The Heap problem is on another machine but communicates to SQL.
November 21, 2013 at 8:16 pm
jcb (11/19/2013)
Question: a select count(1) from inserted ill return the number of inserted rows?just for the record: avoid triggers, they are evil.
avoid to put business logic in the database, let the application to handle it in the appropriate layer.
Just for the record and to emphasize what Gail has already posted, I strongly disagree with both notions.
Poorly written triggers are evil... not all triggers.
Putting certain types of business logic in the database ensures the business logic is carried out when actions are initiated through other means than the application. Sometimes the "appropriate layer" is actually the data layer.
But triggers, cursors, denormalization and other DB tools are misused most of time by developers.
Misuse by people not qualified to use the tools doesn't make the tools bad. A chain saw can cut your arm off but some know how to carve intricate works of art with one.
--Jeff Moden
Change is inevitable... Change for the better is not.
Viewing 15 posts - 1 through 14 (of 14 total)
You must be logged in to reply to this topic. Login to reply