March 31, 2008 at 2:45 pm
Could I be a total noob?
I could of sworn that a trigger gets called once for each record updated.
I'm writing a insert/update trigger on a table that does not have a primary key. A query updates the data - two records. The trigger seems to run only once for that update.
Is this correct? How could I have been doing this for so long and not noticed this?
So, then I query the inserted table in the sp and it has _two_ records. There are two distinct records in my inserted table. Then I run the inserted table thru a cursor for my logic and it works!
This morning i was certain that a trigger runs once per record updated. Is there perhaps a setting that I've missed all this time or some devious condition that I'm hiting?
March 31, 2008 at 3:07 pm
Triggers run once per command, not once per row. I think, but could be mistaken, that they run once per row in Oracle.
It's very important in MS SQL to make all code (triggers, procs, etc.) work on sets of data if at all possible. I would seriously consider removing the cursor and making it a set-based command, if you can.
Is the trigger something you can post here? (Include the table definition too.) We might be able to help with making it set-based.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
March 31, 2008 at 3:34 pm
Ahh, I must be confused with Oracle which would make sense.
The big picture is a bit complicated. I certainly realize set based is better but this db will almost certainly never have more than a few thousand records so I think it should be ok.
Just in general tho, its a cms that has a recursive tree structure with the usual id/parentid fields where each item represents a page [sitepages]. The site url structure is built of the sitepages.pagename field.
So, I keep a field called sitepages.urlpath that is built of the pagenames by a stored procedure that is called whenever either the pagename or the pageid is changed on the trigger to maintain this string. I need this string for quick access when a page is requested.
It just happens for historical reasons that the published and non published sitepages records are in the same table with the same id. (I know! I know! Working on it.) Explaining why one update query updates two records.
So, basically, I do need the trigger to call my stored procedure for each item changed. I don't think this lends to set based processing but please do let me know if I'm wrong.
March 31, 2008 at 4:54 pm
Andre T (3/31/2008)
So, basically, I do need the trigger to call my stored procedure for each item changed.
Absolutely NOT! That's RBAR on steroids and it will crush any hope of any kind of performance in the face of scalability. The best thing to do would be to incorporate your stored procedure into the trigger so that it can process the rows of the INSERTED and DELETED rows using high performance SET BASED techniques. If you really want the sproc to be separate, rewrite it to do SET BASED processing, create a temp table in the trigger, and have the spoc use the temp table as a set of data to operate on.
--Jeff Moden
Change is inevitable... Change for the better is not.
April 1, 2008 at 9:36 am
Andre T (3/31/2008)
Ahh, I must be confused with Oracle which would make sense.The big picture is a bit complicated. I certainly realize set based is better but this db will almost certainly never have more than a few thousand records so I think it should be ok.
And if it does go over a 'couple thousand rows'?
Write stuff optimally from the beginning and you won't have the 'pleasure' of going back and rewriting later. :w00t:
If you like, post the code for the trigger and proc as they now stand. I'm sure someone here will be happy to help you rewrite it
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
April 1, 2008 at 9:46 am
GilaMonster (4/1/2008)
Andre T (3/31/2008)
Ahh, I must be confused with Oracle which would make sense.The big picture is a bit complicated. I certainly realize set based is better but this db will almost certainly never have more than a few thousand records so I think it should be ok.
And if it does go over a 'couple thousand rows'?
Write stuff optimally from the beginning and you won't have the 'pleasure' of going back and rewriting later. :w00t:
If you like, post the code for the trigger and proc as they now stand. I'm sure someone here will be happy to help you rewrite it
I whole heartedly agree with Gail... low volumes of data is no excuse for doing it "wrong" to begin with. It WILL bite you later on and when you can least afford it. I've seen it happen too many times and make a pretty good living at fixing "low volume code".
--Jeff Moden
Change is inevitable... Change for the better is not.
April 1, 2008 at 9:56 am
Yes, it certainly is rbar on steroids and I'm familiar with the concept. But when you say it leads to an unscalable system, you are making a lot of assumptions about my design.
First off, I've mentioned there will never be more than a few thousand records in this particular table. So, its going to be in the milliseconds. Second, this trigger will only run when changes are made in the admin. Thus, it only runs on rare conditions where certain things that don't get changed much change. So thats maybe a few hundred times a day at very worst case and typically in the single or double digits.
And then what does it buy me on the public very high volume side of the system? Well it buys me a set of variables that I can do simple select queries on. Translation - it enables very fast queries where speed is critical. If I didn't do it this way, then I'd have to do other more painful things that would indeed make my system slow.
If you have good links related to people doing similar things - ie maintaining somewhat complicated variables via set based triggers - I'm all eyes.
I have researched myself and I've read the articles that come thru this sites email broadcast. I've concluded that the benefits are lesser for smaller data sizes and also that the variables I require are too complex to maintain without the individual sp call. But please I'll take all the help I can get!
Thanks!
Andre
April 1, 2008 at 10:17 am
Your right Gail, I have nothing to loose by feedback from people who know more than I(except some or even lots humiliation - HA!).
This may look slow and I'll certainly agree that if I had tens or hundreds of thousands of pages, I might think again but its extremely highly unlikely I'll ever get more than 2000.
Thanks!
Andre
Here is the table.
/****** Object: Table [dbo].[sitepages] Script Date: 04/01/2008 09:59:50 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
SET ANSI_PADDING ON
GO
CREATE TABLE [dbo].[sitepages](
[id] [varchar](35) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
[pagename] [varchar](50) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
[title] [varchar](500) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
[parentid] [varchar](35) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
[siteid] [varchar](90) COLLATE SQL_Latin1_General_CP1_CI_AS NOT NULL,
[modifiedby] [varchar](35) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
[modifieddate] [datetime] NULL CONSTRAINT [DF_sitepages_modifieddate] DEFAULT (getdate()),
[breadcrumbs] [varchar](500) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
[urlpath] [varchar](100) COLLATE SQL_Latin1_General_CP1_CI_AS NULL
) ON [PRIMARY]
GO
SET ANSI_PADDING OFF
Here is the trigger
set ANSI_NULLS ON
set QUOTED_IDENTIFIER ON
go
-- =============================================
-- Author:DRE
-- =============================================
ALTER TRIGGER [dbo].[updateSettingsTrigger]
ON [dbo].[sitepages]
FOR INSERT,UPDATE
AS
DECLARE @id varchar(35),
@siteid varchar(100),
@dorun int,
@tmp int
DECLARE itms CURSOR READ_ONLY LOCAL FOR SELECT id, siteid FROM inserted
BEGIN
SET NOCOUNT ON;
--confirm should run this trigger
SELECT @dorun = COUNT(*)
FROM inserted i
INNER JOIN deleted d
ON ((i.id = d.id AND i.siteid = d.siteid)
AND
(i.pagename <> d.pagename OR i.parentid <> d.parentid))
IF @dorun <> 0
BEGIN
OPEN itms
FETCH NEXT FROM itms INTO @id, @siteid
WHILE @@FETCH_STATUS = 0
BEGIN
INSERT INTO temp (ts, str) VALUES (getDate(), @id + ' ' + @siteid)
EXEC dbo.updateSettings @id, @siteid, 1
FETCH NEXT FROM itms INTO @id, @siteid
END
CLOSE itms
DEALLOCATE itms
END
END
Here are the sps
GO
/****** Object: StoredProcedure [dbo].[updateSettings] Script Date: 04/01/2008 10:01:37 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
-- =============================================
-- Author:DRE
-- =============================================
CREATE PROCEDURE [dbo].[updateSettings]
-- Add the parameters for the stored procedure here
@id varchar(35),
@siteid varchar(100),
@wchildren bit
AS
BEGIN
DECLARE @pathStr varchar(255)
DECLARE kids CURSOR local FOR
SELECT id FROM dbo.sitepages
WHERE parentid = @id AND siteid = @siteid
DECLARE @kidid varchar(35)
EXEC dbo.updateSetting @id, @siteid
IF @wchildren = 1
BEGIN
OPEN kids
FETCH NEXT FROM kids INTO @kidid
WHILE @@FETCH_STATUS = 0
BEGIN
EXEC dbo.updateSettings @kidid, @siteid, 1
FETCH NEXT FROM kids INTO @kidid
END
CLOSE kids
DEALLOCATE kids
END
END
GO
/****** Object: StoredProcedure [dbo].[updateSettings] Script Date: 04/01/2008 10:01:37 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
-- =============================================
-- Author:DRE
-- =============================================
CREATE PROCEDURE [dbo].[updateSettings]
-- Add the parameters for the stored procedure here
@id varchar(35),
@siteid varchar(100),
@wchildren bit
AS
BEGIN
DECLARE @pathStr varchar(255)
DECLARE kids CURSOR local FOR
SELECT id FROM dbo.sitepages
WHERE parentid = @id AND siteid = @siteid
DECLARE @kidid varchar(35)
EXEC dbo.updateSetting @id, @siteid
IF @wchildren = 1
BEGIN
OPEN kids
FETCH NEXT FROM kids INTO @kidid
WHILE @@FETCH_STATUS = 0
BEGIN
EXEC dbo.updateSettings @kidid, @siteid, 1
FETCH NEXT FROM kids INTO @kidid
END
CLOSE kids
DEALLOCATE kids
END
END
GO
/****** Object: StoredProcedure [dbo].[updateBreadcrumbStr] Script Date: 04/01/2008 10:02:34 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
-- =============================================
-- Author:DRE
-- =============================================
CREATE PROCEDURE [dbo].[updateBreadcrumbStr]
@id varchar(35),
@siteid varchar(100)
AS
BEGIN
DECLARE @breadcrumb varchar(255)
SELECT @breadcrumb = dbo.getBreadCrumbs(@id, @siteid);
UPDATE dbo.sitePages
SET breadcrumbs = @breadcrumb
WHERE id = @id AND siteid = @siteid;
END
GO
/****** Object: StoredProcedure [dbo].[updateUrlPathStr] Script Date: 04/01/2008 10:02:52 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
-- =============================================
-- Author:DRE
-- =============================================
CREATE PROCEDURE [dbo].[updateUrlPathStr]
-- Add the parameters for the stored procedure here
@id varchar(35),
@siteid varchar(100)
AS
BEGIN
DECLARE @pathStr varchar(255)
SELECT @pathStr = dbo.getPath(@id, @siteid);
UPDATE dbo.sitePages
SET urlpath = @pathStr
WHERE id = @id AND siteid = @siteid;
END
GO
/****** Object: UserDefinedFunction [dbo].[getBreadCrumbs] Script Date: 04/01/2008 10:03:09 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
-- =============================================
-- Author:Dre
-- Create date: 20070928
-- Description:Function used to maintain pathing info for site
-- this function gets info on the current page,
-- then loops up the path in a while loop and concatenates a string that makes up the breadcrumbs.
-- =============================================
CREATE FUNCTION [dbo].[getBreadCrumbs]
(
-- Add the parameters for the function here
@id varchar(35),
@siteid varchar(50)
)
RETURNS varchar(1000) -- varchar(200)
AS
BEGIN
-- Declare the return variable here
DECLARE @parentid varchar(35)
DECLARE @pagename varchar(50)
DECLARE @path varchar(1000) -- varchar(200)
DECLARE @count int
DECLARE @localpath varchar(200)
DECLARE @fdlmtr int
DECLARE @pageid varchar(35)
DECLARE @subsite int
SET @path = ''
SET @count = 0
SELECT
@parentid = parentid,
@pagename = pagename,
@subsite = subSite,
@pageid = id
FROM sitepages
WHERE id = @id AND siteid = @siteid
WHILE @parentid IS NOT NULL AND @parentid <> '' AND @count < 20
BEGIN
IF @count = 0
BEGIN
SET @localpath = dbo.getPath(@id, @siteid)
SET @path = @pagename + ':' + @pageid + ':' + @localpath + '|' + @path
SET @count = @count + 1
END
ELSE IF @subsite = 1
BEGIN
SET @parentid = ''
END
ELSE
BEGIN
SET @localpath = dbo.getPath(@parentid, @siteid)
SELECT @pagename = pagename, @parentid = parentid, @subsite = subSite, @pageid = id FROM sitepages WHERE id = @parentid AND siteid = @siteid
SET @path = @pagename + ':' + @pageid + ':' + @localpath + '|' + @path
SET @count = @count + 1
END
END
RETURN @path
END
USE [fpa]
GO
/****** Object: UserDefinedFunction [dbo].[getPath] Script Date: 04/01/2008 10:03:30 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
-- =============================================
-- Author:Dre
-- Create date: 20070928
-- Description:Function used to maintain pathing info for site
-- =============================================
CREATE FUNCTION [dbo].[getPath]
(
-- Add the parameters for the function here
@id varchar(35),
@siteid varchar(50)
)
RETURNS varchar(200)
AS
BEGIN
-- Declare the return variable here
DECLARE @parentid varchar(35)
DECLARE @pagename varchar(50)
DECLARE @path varchar(200)
DECLARE @count int
DECLARE @fdlmtr int
SET @path = ''
SET @count = 0
SELECT @parentid = parentid, @pagename = pagename FROM sitepages WHERE id = @id AND siteid = @siteid
-- SELECT @parentid = parentid FROM sitepages WHERE id = @id AND siteid = @siteid
-- SELECT @pagename = pagename FROM sitepages WHERE id = @id AND siteid = @siteid
WHILE @parentid IS NOT NULL AND @parentid <> '' AND @count < 20
BEGIN
SET @pagename = REPLACE(@pagename,' ','')
SET @pagename = REPLACE(@pagename,'&','')
SET @path = @pagename + '/' + @path
SET @count = @count + 1
SELECT @pagename = pagename, @parentid = parentid FROM sitepages WHERE id = @parentid AND siteid = @siteid
-- SELECT @pagename = pagename FROM sitepages WHERE id = @parentid AND siteid = @siteid
-- SELECT @parentid = parentid FROM sitepages WHERE id = @parentid AND siteid = @siteid
END
SET @pagename = REPLACE(@pagename,' ','')
SET @pagename = REPLACE(@pagename,'&','')
-- Return the result of the function
SET @path = @pagename + '/' + @path -- getPath(@parentid) + '/' + @pagename
SET@fdlmtr = CHARINDEX('/', @path)
SET @path = RIGHT(@path, LEN(@path) - @fdlmtr)
RETURN @path
END
April 1, 2008 at 10:46 am
Andre T (4/1/2008)
Your right Gail, I have nothing to loose by feedback from people who know more than I(except some or even lots humiliation - HA!).
No, no... no humiliation... just trying to keep you out of trouble.
Just a quick glance kinda tells me you're doing something hierarchical in nature (I just don't have the time to read that much undocumented code ;))
Could you give an over view of what the code is doing for each row in the trigger, please?
--Jeff Moden
Change is inevitable... Change for the better is not.
April 1, 2008 at 11:18 am
Andre T (4/1/2008)
Yes, it certainly is rbar on steroids and I'm familiar with the concept. But when you say it leads to an unscalable system, you are making a lot of assumptions about my design.First off, I've mentioned there will never be more than a few thousand records in this particular table. So, its going to be in the milliseconds. Second, this trigger will only run when changes are made in the admin. Thus, it only runs on rare conditions where certain things that don't get changed much change. So thats maybe a few hundred times a day at very worst case and typically in the single or double digits.
In my experience, there's really no such thing as a cursor-based update operation on "a few thousand" rows being done "in the milliseconds". I doubt you'd be hearing much from Gail or Jeff if those were to be that fast. In the milliseconds = a handful at most.
To a cursor - a few thousand is a lot, IMO.
Interestingly enough - I just benchmarked a set-based update against a cursor-based one a couple of days ago. Just thought I'd share the results with you (I've seen this repeat over and over again).
Rows affected 1000100002000030000 50000
Cursor-Based update (ms) 55702157934259769089561647296
Set-based update (ms) 453 4486 5670 8486 11236
Ratio Cursor/Set: 12.295848.103775.1280107.1124146.6088
And well - I would have gone to 100,000 rows just for giggles, but it just never finishes on the cursor side (it ran over 2 hours before I killed it). The set-based? 17 seconds, (45 seconds for 1M).
Don't get me wrong - I understand fully that the reason for the code is needed: preprocessing the path is no doubt important. It is just that on that day where EVERY page you have gets updated, the cursor version of the trigger is going to have a bad day....
Just trying to keep you from buying into the "it will never grow" hoax. Anything I've ever had that I was assured would "never" grow, well - it did and "they" lied.
----------------------------------------------------------------------------------
Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?
April 1, 2008 at 11:44 am
Andre, I must be missing something. Your updateSettings proc doesn't seem to actually do anything except call itself recursively. I don't see any update/insert/delete commands in it. What am I missing?
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
April 1, 2008 at 11:57 am
Matt,
Awesome. Can you post the test code? That sure sounds like a "keeper" to me.
--Jeff Moden
Change is inevitable... Change for the better is not.
April 1, 2008 at 12:13 pm
Jeff Moden (4/1/2008)
Matt,Awesome. Can you post the test code? That sure sounds like a "keeper" to me.
You've already seen it... right here (sorry - forgot to put in the cross-ref earlier):
http://www.sqlservercentral.com/Forums/Topic475800-149-2.aspx#bm477390
It's the modified RegexReplace function for running multiple replace ops. I even "improved" the cursor process from the initial posting (some of the replacements are getting done over and over and over again).
----------------------------------------------------------------------------------
Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?
April 1, 2008 at 12:15 pm
My bad, I posted updatesettings twice.
Below is the code for updatesetting which is called from updatesettings (singular vs plural) near the top.
Just to give some flow, I've kept most of the sps small for readability.
The site is represented by a recursive tree structure with the id/parentid relationship in the sitepages table. There is a field in the sitepages table called pagename. The tree structure and the pagename represent the site url structure.
So, if I have a page with pagename "About Us" which has another page called "Directors" linked to it below by the parentid field, then the url link to thatpage will be /AboutUs/Directors/. The whole discussion is about maintaining that field in the db for quick lookup. Also, each page has a breadcrumbs string whcih would be vaguely like " ". That is maintained in a similar manner.
So, the trigger watches for two fields being updated - parentid and pagename. When they are updated, they call the sp updateSettings which recursively calls updateSetting which uses the scalar valued functions update updateBreadcrumbStr and updateURLPathStr to generate both the urlpath value and the breadcrumbs value. Both are functions that create a string by climbing up the parentid fields of the sitepages table.
I'll load up the site with some fake data and do some testing. I'll post my results soon.
Thanks
Andre
GO
/****** Object: StoredProcedure [dbo].[updateSetting] Script Date: 04/01/2008 11:57:16 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
-- =============================================
-- Author:DRE
-- =============================================
CREATE PROCEDURE [dbo].[updateSetting]
-- Add the parameters for the stored procedure here
@id varchar(35),
@siteid varchar(100)
AS
BEGIN
EXEC dbo.updateBreadcrumbStr @id, @siteid
EXEC dbo.updateURLPathStr @id, @siteid
END
April 1, 2008 at 2:34 pm
Matt Miller (4/1/2008)
Jeff Moden (4/1/2008)
Matt,Awesome. Can you post the test code? That sure sounds like a "keeper" to me.
You've already seen it... right here (sorry - forgot to put in the cross-ref earlier):
http://www.sqlservercentral.com/Forums/Topic475800-149-2.aspx#bm477390
It's the modified RegexReplace function for running multiple replace ops. I even "improved" the cursor process from the initial posting (some of the replacements are getting done over and over and over again).
O-o-o-h... THAT code. Thanks Matt!
--Jeff Moden
Change is inevitable... Change for the better is not.
Viewing 15 posts - 1 through 15 (of 22 total)
You must be logged in to reply to this topic. Login to reply