April 1, 2008 at 3:06 pm
Okay. Now I can follow what's going on.
You have a trigger that uses a cursor to call a proc that uses a cursor to call another proc that calls two scalar functions that each use While loops to resolve a hierarchy.
It should work, but I'm pretty sure it can all be replaced with a single recursive CTE, a couple of string functions, and one update command.
If you really don't care about performance and maintainability, what you have will do. The next person who has to work with it, if anyone, might feel a strange urge to hunt you down and pummel you with a book on set theory, but the code should compile, run and get the right end result. 🙂
- 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 4:11 pm
I tested the worst case use of that code function with test data equivalent to a site with 6000 records. Resetting all of the urlpaths and breadcrumbs which is an superadmin thing took 37 seconds on my 2 year old laptop.
I'm switching some things around such that that will theoretically be equivalent to a 12000 page site. Most users will call a subset of that typically with 10 to 100 records. So, its acceptably fast but definetly room for improvement.
GS Nicely put
You have a trigger that uses a cursor to call a proc that uses a cursor to call another proc that calls two scalar functions that each use While loops to resolve a hierarchy.
I looked up the CTE sql server item and its certainly interesting - tho not easily ported to other dbs. I can see where I could use it to update the getpath and getbreadcrumbs functions and make them a bit more readable and maybe faster but I'm missing as to where I could use it to sweep up an item and all its children and then update two of its fields in one set based swoop? What did I miss?
GS, you wrote:
If you really don't care about performance and maintainability, what you have will do.
Why did you write that? Im obviously interested as you might infer from my posting the code and answering questions about it.
As for the maintainability, which I translate to readability, it seems like my sps are very simple and easy to read - I take care to separate the individual functionality into individual methods. Only the getbreadcrumbs and getpath functions need a second glance. Certainly, I don't pass myself as a dba so please let me know what you mean?
April 1, 2008 at 4:33 pm
Try this:
create trigger dbo.updateSettingsTrigger
after update
as
if update(pagename) or update(parentid)
begin
;with CTE (ID, ParentID, Pagename, Breadcrumbs, URLPath) as
(select id, parentid, pagename, breadcrumbs, urlpath
from inserted
union all
select sitepages.id, sitepages.parentid, sitepages.pagename,
cte.breadcrumbs + (breadcrumb string),
cte.urlpath + '/' + sitepages.pagename
from dbo.sitepages
inner join cte
on sitepages.parentid = cte.id)
update dbo.sitepages
set breadcrumbs = cte.breadcrumbs,
urlpath = cte.urlpath
from cte
left outer join inserted
on cte.id = inserted.id
where inserted.id is null
You'll need to replace the part where I put "(breadcrumb string)". I'm not clear on what you have in that column, but it will simply be whatever you would add at the very end of the string.
- 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 4:49 pm
Jeff Moden (4/1/2008)
O-o-o-h... THAT code. Thanks Matt!
Well - I'd write another but I'm already at my 1-cursor per week limit...:)
----------------------------------------------------------------------------------
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 4:50 pm
Gosh, I hate the idea of replacing one form of recursion with another, but the recursive CTE will at least handle a set even if it is hidden RBAR.
--Jeff Moden
Change is inevitable... Change for the better is not.
April 2, 2008 at 12:52 pm
Jeff Moden (4/1/2008)
Gosh, I hate the idea of replacing one form of recursion with another, but the recursive CTE will at least handle a set even if it is hidden RBAR.
If you can think of a better way to get the end result he's looking for, go for it. I'm just looking at replacing:
A trigger that uses a cursor to call a proc
Which uses a cursor to call another proc
Which calls two UDFs
Which both use While Loops to build strings
Which returns data to the calling proc, which updates one row in the main table at a time
The CTE is RBAR, I can't see a way around that, but the update is a set.
- 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 2, 2008 at 2:54 pm
Oh, I agree... first step towards some sort of performance is to reduce the number of cursors and UDFs to zero by using the rCTE.
Second step would be to think of how to remove the rCTE but then I'd be rewritng his whole app...
--Jeff Moden
Change is inevitable... Change for the better is not.
Viewing 7 posts - 16 through 22 (of 22 total)
You must be logged in to reply to this topic. Login to reply