Creating cursor with embedded where clause
-
We are migrating a db from Sybase SQLAny to SS2K5. One SQLAny developer used the the following technique integrate a "WhereClause" string parameter into a procedure cursor. Here's the gist:
CREATE PROCEDURE DBO.CancelProcess @WhereClause varchar(max) as....
declare @Select1 varchar(max)
set @Select1=' SELECT Projects.ProjectID, Projects.TargetStartDate, Projects.TargetFinishDate, Projects.StatusCode
from dbo.Projects WHERE ' + @WhereClause
begin
declare curCancel insensitive scroll cursor for @Select1
.....
The error we get is:
Incorrect syntax near '@Select1'
I've tried a few techniques that I know, like declaring the cursor in a Execute statement:
set @Select1= 'SET @curCancel = CURSOR scroll FOR SELECT Projects.ProjectID, Projects.TargetStartDate from DBO.Projects WHERE ' + @WhereClause ;
Execute @Select1
I'm not getting anywhere. What am I missing?
Takauma
-
This isn't Oracle... stop using cursors.
--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) -
What are you trying to do with the cursor? Post the entire code if possible, there's probably a better way.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverabilityWe walk in the dark places no others will enter
We stand on the bridge and no one may pass -
Thank you. Down with cursors
Cheers,CrispinI can't die, there are too many people who still have to meet me!It's not a bug, SQL just misunderstood me!
-
I read "Down with cursors" Fine, but these individuals are suggesting any alternatives. How about some suggestions?
Takauma
-
Sure. Please post the entire piece of code, especially what's inside the cursor loop and I'll give it a shot.
Without knowing what you're doing with the cursor, it's near impossible to suggest an alternative.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverabilityWe walk in the dark places no others will enter
We stand on the bridge and no one may pass -
Like a couple of folks have asked already... post the rest of the code that uses the cursor and describe what the code is supposed to do. We're pretty good at writing high speed alternatives to cursors, but not reading minds
--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) -
Okay okay! Here is the code, which now works since I applied the solutionm that Pieter Huiberts provided in the initial reply to this thread. I normally try to glean it down to the specific issue at hand, this time I left all the code in.
Remember, the question now is: "What are the T-SQL alternatives to the typical declare/open cursor method?"
Thanks, Joel
CREATE PROCEDURE "DBO"."ProjectCancelProcess"(@WhereClause varchar(max), @NewStatus varchar(20)) as
begin
-- This procedure will perform additional Post Processing based on the settings in the StatusRules table.
declare @Select1 varchar(max) ,
@ProjectID integer,
@TargetStartDate date,
@TargetFinishDate date,
@Amount decimal(19,4),
@CancelAmount decimal(19,4),
@OldStatus varchar(20)
--
if @NewStatus = 'Cancelled'
begin
set @CancelAmount=0
set @Select1=' SELECT Projects.ProjectID, Projects.TargetStartDate, Projects.TargetFinishDate, Projects.StatusCode
from DBO.Projects join DBO.OwnerInformation on Projects.OwnerCode = OwnerInformation.OwnerCode WHERE ' + @WhereClause
begin
declare curCancel insensitive scroll cursor for @Select1
open curCancel
fetch next from curCancel into @ProjectID , @TargetStartDate, @TargetFinishDate , @OldStatus
While @@fetch_status = 0
begin
if exists(select 1 from DBO.StatusRules where FromStatusCode = @OldStatus and ToStatusCode = @NewStatus and
PostProcess = 1)
begin
--
-- insert the zero Forecast record if it does not exist..
select top 1 @Amount = Amount from DBO.ProjectForecast where ProjectID = @ProjectID order by
EnteredDate desc
if isnull(@Amount,1) != 0
insert into DBO.ProjectForecast( ProjectID,Amount,TargetStartDate,TargetEndDate,SystemCode) values(
@ProjectID,@CancelAmount,@TargetStartDate,@TargetFinishDate,@NewStatus)
end
--
-- Lock all the Project records on Cancel
--
update
DBO.ProjectChangeID
set ProjectChangeID.Lockflag = 1,ProjectChangeID.StatusCode = 'Cancelled'
from DBO.ProjectChangeID join
Projects on Projects.ProjectID = ProjectChangeID.ProjectID join
OwnerInformation on Projects.OwnerCode = OwnerInformation.OwnerCode where
ProjectChangeID.LockFlag != 1 and ProjectChangeID.StatusCode != 'Cancelled' and
Projects.ProjectID = @ProjectID
--also do the header
-- but only if the Project is not already locked
update DBO.Projects
set Projects.Lockflag = 1
from DBO.Projects join
DBO.ProjectChangeID on ProjectChangeID.ProjectID = Projects.ProjectID join
DBO.OwnerInformation on OwnerInformation.OwnerCode = Projects.OwnerCode where Projects.LockFlag != 1 and Projects.ProjectID = @ProjectID
end
--
fetch next from curCancel into @ProjectID , @TargetStartDate, @TargetFinishDate , @OldStatus
end
close curCancel
deallocate curCancel
end
end
Takauma
-
Working on this but need some info... what column names will you use in the @WhereClause variable for this proc only? Please give an example of what you will pass in the variable...
Also remember... my request was to also post a purpose for the code...
-- This procedure will perform additional Post Processing based on the settings in the StatusRules table.
... just doesn't get it (WHAT kind of additional post processing???)... fortunately, I understand what the overall code is doing so you don't need to go there.
Don't forget about what I asked for in the first paragraph, please...
--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) -
Never mind about the @WhereClause... was going to try to make it so you didn't need the dynamic SQL, but I can't wait for it...
One of the common methods for cursor replacement is the use of a temp table as I've done below... but most folks make the horrible mistake of keeping the rest of the looping code and that's no better than a cursor (sometimes, it's worse). If you create a temp table to hold a full result set, use it with set-based code... not more WHILE loops...
While we're at it... you might want to adopt some readability and formatting standards which should include documentation standards.
Obviously, I've NOT tested this code because I just don't have your tables and data... but it should be pretty close... READ the comments because I think the original code had a couple of unnessary joins (unless they're just being used to prove that someone owned the projects... ie. NOT NULL owership)... I also don't think you need the 0 amount detectors in the code... just because a project has no cost associated with it, doesn't mean that you shouldn't be able to cancel it... but I kept it in the code because that's the way you wrote it.
CREATE PROCEDURE dbo.ProjectCancelProcess /********************************************************************************************************************** Purpose: This code accepts the criteria to be used in a WHERE clause and a new status. If the new status is 'Cancelled', the proc assigns the 'Cancelled' status to the ProjectForecast (zeros out amount as well), the Project, and the ProjectChangeID tables as well as setting the Lockflag.
Revisions: Rev 01 - 07/04/2007 - Rewritten to NOT use cursors or loops **********************************************************************************************************************/ --===== Define the input parameters ( @pWhereClause VARCHAR(MAX), @pNewStatus VARCHAR(20)) ) AS
--===== If the new status is not cancelled, nothing to do here... exit early... IF @pNewStatus <> 'Cancelled' RETURN
--===== Declare local variables DECLARE @SQLInsert VARCHAR(MAX)
--===== Create a temp table to hold common data to replace the cursor CREATE TABLE #Common ( ProjectID INT PRIMARY KEY, Amount DECIMAL(19,4), TargetStartDate DATE, TargetEndDate DATE, OwnerCode VARCHAR(100), --Make this match the data type of OwnerCode because not enough info to figure it out. SystemCode VARCHAR(20) )
--===== Insert cancellation rows (Amount = 0) into the temp table for each project found by the input criteria (@pWhereClause) -- This requires that the old status and new status meet the requirements laid out in the rules table. -- The ProjectID will be used for the other updates, as well. SET @SQLInsert = ' INSERT INTO #Common (ProjectID ,Amount ,TargetStartDate ,TargetEndDate ,OwnerCode , SystemCode) SELECT p.ProjectID, 0 AS Amount ,p.TargetStartDate ,p.TargetFinishDate ,p.OwnerCode, ''' + @pNewStatus + ''' FROM dbo.Projects p, dbo.ProjectForecast pf, (--==== Derived table "maxpf" finds latest entry date for each project SELECT ProjectID, MAX(EnteredDate) AS MaxEnteredDate FROM dbo.ProjectForecast ) maxpf, dbo.StatusRules sr WHERE p.ProjectID = pf.ProjectID AND pf.ProjectID = maxpf.ProjectID AND pf.EnteredDate = maxpf.MaxEnteredDate AND p.StatusCode = sr.FromStatusCode AND sr.ToStatusCode = ''' + @pNewStatus + ''' AND sr.PostProcess = 1 AND (pf.Amount <> 0 OR pf.Amount IS NULL) AND ' + @pWhereClause
EXEC (@SqlInsert)
--===== Insert the new forecast rows for cancelled projects INSERT INTO dbo.ProjectForecast (ProjectID, Amount, TargetStartDate, TargetFinishDate, SystemCode) SELECT ProjectID, Amount, TargetStartDate, TargetFinishDate, SystemCode FROM #Common
--===== Lock the project rows on cancel where not already locked UPDATE pc SET pc.Lockflag = 1, pc.StatusCode = c.SystemCode FROM dbo.ProjectChangeID pc, #Common c, dbo.OwnerInformation o -- Is this really needed??? WHERE pc.ProjectID = c.ProjectID AND c.OwnerCode = o.OwnerCode -- Is this really needed??? AND pc.Lockflag <> 1 AND pc.StatusCode <> c.SystemCode
--===== Lock the headers, too, where not already locked UPDATE p SET Lockflag = 1, StatusCode = c.SystemCode --Added this 'cause I thought it was necessary FROM dbo.Projects p, #Common c, dbo.OwnerInformation o -- Is this really needed??? WHERE p.ProjectID = c.ProjectID AND c.OwnerCode = o.OwnerCode -- Is this really needed??? AND p.Lockflag <> 1 AND p.StatusCode <> c.SystemCode
RETURN
--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) -
Kinda busted a hump trying to help you get rid of the cursor on this one, Joel... any feedback?
--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) -
Sorry for the delay Jeff:
I had thought my reply was already posted, but that most have been re some e other issue.
The SP that spawned my original post was eventually handled by another person in my group, so I have yet to apply the solution you laid out.
However... I did study it in detail. Many thanks!
Feedback: It really more like "d'oh!". We use this technique with statement level triggers whenever it fits, (in Sybase SQLA anyway), updates and inserts using joins with the "deleted" and/or "inserted" recordsets. Elegant.
I suppose that I was so blind to it, shows just how "iterative" we can get with procedures. I gotta take a step back now and then!
Joel
Takauma
-
LOL... yeup... been in that same spot once or twice myself. Anyway, thanks for getting back to me.
--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)
Viewing 14 posts - 1 through 13 (of 13 total)
You must be logged in to reply to this topic. Login to reply