August 23, 2008 at 6:16 pm
Can someone help to how to remove the cursor. when i look at the stored proc most of the stored proc are using cursor. I also figuered out there is high fragmentation on most to of the table structure. i have already setup the maintainance plan for that. But looking at the stored proc it is making me nervous. I need help what should i look on stored proc to improve performances. can some one helped me out in detail.
thanks
Sagar
August 24, 2008 at 1:49 am
Could you perhaps post the query? It's hard to say how to improve something without some code to refer to.
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
August 24, 2008 at 9:00 am
Usually a while loop easily removes it, but perhaps you can make something set based.
However we would like to see you do some work, show some effort and ask questions about what you've done or are thinking of.
August 24, 2008 at 9:16 am
Sagar (8/23/2008)
Can someone help to how to remove the cursor. when i look at the stored proc most of the stored proc are using cursor. I also figuered out there is high fragmentation on most to of the table structure. i have already setup the maintainance plan for that. But looking at the stored proc it is making me nervous. I need help what should i look on stored proc to improve performances. can some one helped me out in detail.thanks
Sagar
I've seen lots of folks try to salvage code that have cursors in it. A few are able to pull it of with some degree of success depending on the use of the cursor. But, the basic thought process behind cursor code is, as you know, mostly flawed to begin with.
I've seen the best successes with folks that figure out what the overall process of the proc does, and simply rewrite it from the ground up never looking back at the original code...
... if done correctly, it'll definitely be worth it.
There will be those (sorry Steve) that say to convert the cursor to a temp table and While loop... that is probably the worst thing you can do because it's nothing more than the equivalent of a read only, forward only cursor which won't save anything. You have to figure out the correct set based logic for the cursors and the surrounding code. A stored proc that handles only one row will also keep you from doing that... all such sprocs must be replaced. And, don't forget to fix any triggers the same way.
--Jeff Moden
Change is inevitable... Change for the better is not.
August 24, 2008 at 10:19 am
code looks like below... also have another question as well... there are lots of temp table inside stored proc.. i was thinking of removing temp table and using table variable instate.. to avoid recompialation... is that will be a good idea or any suggestions.
BEGIN
SET NOCOUNT ON;
DECLARE
@PlcForecastIdINT
,@PlcForecastQuarterINT
,@PlcForecastYearINT
,@TempNoteVARCHAR(255)
,@NoteVARCHAR(255)
,@QuarterYearValueVARCHAR(5)
-- Declare table to hold the details data with months translated to quarters
DECLARE @PlcForecastTable TABLE
([PlcForecastDetailId]INT
,[PlcForecastId]INT
,[PlcForecastQuarter]INT
,[PlcForecastYear]INT
,[PlcForecastValue]DECIMAL(10,2)
,[Note]VARCHAR(255))
-- Declare table to hold notes translated to one by quarter
DECLARE @PlcForecastTableNotes TABLE
([PlcForecastId]INT
,[PlcForecastQuarter]INT
,[PlcForecastYear]INT
,[Note]VARCHAR(255))
-- Translate the data
INSERT INTO @PlcForecastTable
([PlcForecastDetailId]
,[PlcForecastId]
,[PlcForecastQuarter]
,[PlcForecastYear]
,[PlcForecastValue]
,[Note])
SELECT HorizonDetailID, HorizonID,
CASE LEFT((HorizonSpan), (CHARINDEX('''', HorizonSpan)-1))
WHEN 'January' THEN 1
WHEN 'February' THEN 1
WHEN 'March' THEN 1
WHEN 'April' THEN 2
WHEN 'May' THEN 2
WHEN 'June' THEN 2
WHEN 'July' THEN 3
WHEN 'August' THEN 3
WHEN 'September' THEN 3
WHEN 'October' THEN 4
WHEN 'November' THEN 4
WHEN 'December' THEN 4
END AS HorizonQuarter,
'20' + RIGHT(HorizonSpan,2) AS PlcForecastYear,
HorizonValue, Note
FROM [RMTwData].[dbo].[RMT_IPAHorizonDetail]
WHERE HorizonID IN (SELECT DISTINCT PlcForecastId FROM RMT.dbo.tblPlcForecasts)
-- Translate the data for the notes table
INSERT INTO @PlcForecastTableNotes(
[PlcForecastId]
,[PlcForecastQuarter]
,[PlcForecastYear]
,[Note])
SELECT PlcForecastId, PlcForecastQuarter
,PlcForecastYear, MAX(Note)
FROM @PlcForecastTable
WHERE Note IS NOT NULL
AND Note <> ''
GROUP BY PlcForecastId, PlcForecastQuarter,PlcForecastYear
-- Get cursor with denormalized notes data concatenate and update in normalized table
DECLARE DenormCursor CURSOR FAST_FORWARD FOR
SELECT [PlcForecastId]
,[PlcForecastQuarter]
,[PlcForecastYear]
,[Note]
FROM @PlcForecastTableNotes
--
OPEN DenormCursor
FETCH FROM DenormCursor
INTO
@PlcForecastId
,@PlcForecastQuarter
,@PlcForecastYear
,@Note
WHILE @@FETCH_STATUS = 0
BEGIN
SELECT @QuarterYearValue = 'Q' + CAST(@PlcForecastQuarter AS VARCHAR) + RIGHT(CAST(@PlcForecastYear AS VARCHAR), 2) + ': '
SELECT @TempNote = @QuarterYearValue + ' ' + LEFT(@Note, 248) + '; '
UPDATE tblPlcForecasts
SET notes = LEFT((ISNULL(notes,'') + @TempNote), 255)
WHERE PlcForecastId = @PlcForecastId
FETCH NEXT FROM DenormCursor
INTO
@PlcForecastId
,@PlcForecastQuarter
,@PlcForecastYear
,@Note
END -- Cursor Loop
CLOSE DenormCursor
DEALLOCATE DenormCursor
August 24, 2008 at 10:51 am
It appears that you are attempting to take monthly forecasts and summarize to quarterly with the monthly notes concatenated into a single column.
Please read Jeff Moden's article titled "Cross Tabs and Pivots, Part 1" for all of the details at http://www.sqlservercentral.com/articles/T-SQL/63681/.
SELECT HorizonDetailID, HorizonID
, HorizonQuarter , PlcForecastYear, HorizonValue
, 'Q1' + RIGHT(CAST(PlcForecastYear AS VARCHAR), 2) + ': '
+MAX( CASE WHEN HorizonMonth = 'January' then COALESCE( Note + ';', '') else '' end )
+MAX( CASE WHEN HorizonMonth = 'February' then COALESCE( Note + ';', '') else '' end )
+MAX( CASE WHEN HorizonMonth = 'March' then COALESCE( Note + ';', '') else '' end )
-- and so forth for the subsequent quarters.
FROM(
SELECT HorizonDetailID, HorizonID
,LEFT((HorizonSpan), (CHARINDEX('''', HorizonSpan)-1)) as HorizonMonth
, CASE LEFT((HorizonSpan), (CHARINDEX('''', HorizonSpan)-1))
WHEN 'January' THEN 1
WHEN 'February' THEN 1
WHEN 'March' THEN 1
WHEN 'April' THEN 2
WHEN 'May' THEN 2
WHEN 'June' THEN 2
WHEN 'July' THEN 3
WHEN 'August' THEN 3
WHEN 'September' THEN 3
WHEN 'October' THEN 4
WHEN 'November' THEN 4
WHEN 'December' THEN 4
END AS HorizonQuarter,
'20' + RIGHT(HorizonSpan,2) AS PlcForecastYear
,HorizonValue
,Note
FROM [RMTwData].[dbo].[RMT_IPAHorizonDetail]
WHEREHorizonID IN (SELECT DISTINCT PlcForecastId FROM RMT.dbo.tblPlcForecasts)
) as ForCastMonthly
group by HorizonDetailID, HorizonID
, HorizonQuarter , PlcForecastYear, HorizonValue
SQL = Scarcely Qualifies as a Language
August 24, 2008 at 11:23 am
Sagar (8/24/2008)
code looks like below... also have another question as well... there are lots of temp table inside stored proc.. i was thinking of removing temp table and using table variable instate.. to avoid recompialation... is that will be a good idea or any suggestions.
We can probably eliminate the Temp tables without having to use any (ugh!) Table variables... and Carl is correct, take at look at the article he pointed you to.
It appears that you're getting the data from just one table (RMTwData.dbo.RMT_IPAHorizonDetail)... if you really want some great help, post the CREATE TABLE statment for that table AND some ready-to-load data using the methods outlined in the link in my signature below. 🙂 We can probably do this in a single set-based cross tab query... kinda like Carl did but maybe even shorter. :w00t:
--Jeff Moden
Change is inevitable... Change for the better is not.
August 24, 2008 at 2:38 pm
Jeff Moden (8/24/2008)
I've seen lots of folks try to salvage code that have cursors in it. A few are able to pull it of with some degree of success depending on the use of the cursor. But, the basic thought process behind cursor code is, as you know, mostly flawed to begin with.I've seen the best successes with folks that figure out what the overall process of the proc does, and simply rewrite it from the ground up never looking back at the original code...
This is the same conclusion that I have come to over the past few months, Jeff. I would much rather rewrite a query straight from the Specs, than have to rewrite a cursor proc from the code alone. Most of the time is spent just trying to figure out what the dang cursor is trying to do in the first place. :angry: Argggh!
The funniest defense of cursors I have read in the last 6 months has got to be "because they are easier to read"! :w00t: Cursors are one of the best examples of write-only code ever.
[font="Times New Roman"]-- RBarryYoung[/font], [font="Times New Roman"] (302)375-0451[/font] blog: MovingSQL.com, Twitter: @RBarryYoung[font="Arial Black"]
Proactive Performance Solutions, Inc. [/font][font="Verdana"] "Performance is our middle name."[/font]
August 24, 2008 at 2:47 pm
Thanks Everybody...
Sagar
August 24, 2008 at 3:27 pm
Sagar (8/24/2008)
Thanks Everybody...Sagar
You're welcome... but if you post the CREATE TABLE and data that I suggested, you might even get a tested example to play with... 😉
--Jeff Moden
Change is inevitable... Change for the better is not.
August 24, 2008 at 3:36 pm
rbarryyoung (8/24/2008)
This is the same conclusion that I have come to over the past few months, Jeff. I would much rather rewrite a query straight from the Specs, than have to rewrite a cursor proc from the code alone. Most of the time is spent just trying to figure out what the dang cursor is trying to do in the first place. :angry: Argggh!
Heh... I have an ever better reason... who says the code actually does what the spec said it was supposed to do? Developer could have taken a short cut or just made a mistake somewhere... 😛
--Jeff Moden
Change is inevitable... Change for the better is not.
August 25, 2008 at 4:51 am
Use of while loop is equivalent of a read only, forward only cursor which won't save anything. SET based approch is the best way to remove the cursor from your query. However it depends on business need. if need is just to transform the data then it's is very easy to remove the use of cursor but if need is to transform the data and sametime inserting the data in another tables (for example bases on some condition you need to add master data and then use it ) in that condition SET based approch does not help us .
//sandeep
August 25, 2008 at 7:24 pm
Hi, It is seems to be very help full to trouble the issue.. i really thanks for that.
i also have have another question.. code is right below the text. what i am doing here is finding all superusers that aren't already in the tblUsers( which is all users) and give them the same properties as Permanent user. problem is that duplicate the users they are in superuser. Does any one have any i dead if there is some thing wrong here. I know it is hard to read someon'e code but if any suggestion that will be appreciate.
DECLARE @AllDivisions int
DECLARE @userid int
DECLARE @AdminId int
SET NOCOUNT ON
SET @AllDivisions = 0
SET @AdminID = dbo.fnCommon_GetAdminUserID();---- thing is nothing but a static numeric value
DECLARE @TempNewUsers TABLE ([UserID] int NULL, [aaID] [varchar](8), [IdSid] [varchar](8), [FirstName] [varchar](50), [LastName] [varchar](50), [MiddleInitial] [varchar](1), [DomainAddress] [varchar] (80))
DECLARE @TempNewUsersNullOnly TABLE ([UserID] int NULL, [aaID] [varchar](8), [IdSid] [varchar](8), [FirstName] [varchar](50), [LastName] [varchar](50), [MiddleInitial] [varchar](1), [DomainAddress] [varchar] (80))
INSERT INTO @TempNewusers
SELECT Null as UserId, aaid, IdSid, FirstName, LastName, MiddleInitial, DomainAddress
FROM ADMIN_SuperUsers
-- Insert the nulls into a special table. Could do everything below with a well written query
-- instead of another temp table, but this table is cheap and it keeps the queries below cleaner
INSERT INTO @TempNewusersNullOnly
SELECT UserId, aaid, IdSid, FirstName, LastName, MiddleInitial, DomainAddress
FROM @TempNewUsers
WHERE UserID IS NULL
-- Set the User Id in our temp table for those users already in CDIS
UPDATE @TempNewUsers
SET UserId = u.UserID
FROM @TempNewUsers tu
JOIN tblUsers u
ON tu.aaid = u.aaid
-- Create new users for those with NULL user IDs (which means they aren't in CDIS)
-- For now based on Admin user
INSERT INTO tblUsers
(aaid, IdSid, FirstName, MiddleInitial, LastName, CostCenterID, DivisionID, SiteID, GLFunctionAreaID, BadgeType, ProfitCenterID, DomainAddress, BuildingID, NewHire, IsActive, ChangedDate, ChangedUserID)
(
SELECT tu.aaid, tu.IdSid, tu.FirstName, tu.MiddleInitial, tu.LastName, u.CostCenterID, u.DivisionID, u.SiteID, u.GLFunctionAreaId, u.BadgeType, u.ProfitCenterID, tu.DomainAddress, u.BuildingId, u.NewHire, u.IsActive, GetDate(), @AdminID
FROM @TempnewUsersNullOnly tu
JOIN
(
SELECT * FROM tblUsers WHERE Userid = (SELECT TOP 1 Userid FROM tblUsers WHERE LastName = 'aaa' and FirstName = 'bbb')
) u
ON 1=1
)
UPDATE @TempNewUsers
SET UserId = u.UserID
FROM @TempNewUsers tu
JOIN tblUsers u
ON tu.aaid = u.aaid
-- Create a cursor to run another script that scriopt will get a new user and put in to the certain division
DECLARE SuperUserCursor CURSOR READ_ONLY FOR
SELECT u.UserId
FROM tblUsers u
WHERE aaid IN
(SELECT aaid FROM ADMIN_SuperUsers)
OPEN SuperUserCursor
FETCH FROM SuperUserCursor
INTO @userid
WHILE @@FETCH_STATUS = 0
BEGIN
PRINT 'Executing PutUserInDivision UserID = ' + cast(@UserID as varchar(10))
-- For each user in the cursor, execute this stored proc
exec PutUserInDivision @user-id, @AllDivisions
FETCH NEXT FROM SuperUserCursor
INTO @userid
END -- SuperUserCursor loop
CLOSE SuperUserCursor
DEALLOCATE SuperUserCursor
END
August 25, 2008 at 9:54 pm
sandeep kumar (8/25/2008)
Use of while loop is equivalent of a read only, forward only cursor which won't save anything. SET based approch is the best way to remove the cursor from your query. However it depends on business need. if need is just to transform the data then it's is very easy to remove the use of cursor but if need is to transform the data and sametime inserting the data in another tables (for example bases on some condition you need to add master data and then use it ) in that condition SET based approch does not help us .//sandeep
It could be that I'm missing something, but INSERT/SELECT does just fine in that area.
--Jeff Moden
Change is inevitable... Change for the better is not.
Viewing 14 posts - 1 through 13 (of 13 total)
You must be logged in to reply to this topic. Login to reply