August 17, 2011 at 9:47 am
Originally I had this curosr desgined to do this by individual and it worked fine, but then they also want to be able to do a group of personnel so i changed to grab all the SSN's of personnel and then roll through the records and update. The problem I am having is that its only grabbing the first record and repeating it indefinetly, can't for the life of me figure this out. It pulls the data correctly but the @execsql statment never changes.
declare @Personnel varchar(9), @execsql nvarchar (4000), @answer varchar(6), @dtlogged varchar(30), @strRemarks varchar(1000), @strLogged varchar(100),
@MPDV varchar(10), @QId int, @Datenow as varchar(35)
set @Datenow = cast(GETDATE() as varchar(max))
Declare Pers Cursor For
Select Distinct intpersonnelId from tblSrpEventDataHistory where intSrpAttendId in
(select intSrpAttendId from tblSRPAttendance where intEventId = @EventId) order by intPersonnelID
OPEN Pers
FETCH Next From Pers INTO @Personnel
WHILE @@FETCH_STATUS = 0
begin
Declare MPDV Cursor FOR
Selected.intquestionId,
q.intMPDVID,
cast(Case When ed.intAnswer = 0 THEN 'GO' WHEN ed.intAnswer = 1 THEN 'NO GO' ELSE 'NO GO' END as varchar(6)),
ed.dtLogged,
ed.strRemarks,
ed.strLogged
FromtblSRPEventDataHistory as ed INNER JOIN
tblSRPQuestion as q on q.intQuestionId = ed.intQuestionId
Whereq.intMPDVID IS NOT NULL AND intPersonnelID = @Personnel
Order by intAnswer
OPEN MPDV
FETCH Next From MPDV INTO @QId,@MPDV,@answer,@dtLogged,@strRemarks,@strLogged
WHILE @@FETCH_STATUS = 0
BEGIN
set @execsql = 'update OPENQUERY(RCASDBOR, ''Select STA, DT_TM_COMPL, NOTES, LAST_UPDT_NM, LAST_UPDT_DT FROM DOIM_RO2.CHECKLIST
WHERETASK_CTRL_SEQ_ID = '+ @MPDV + ' AND
UNIT_Pers_ID = '+ @Personnel +''')
SETSTA = ''' + @answer + ''',
DT_TM_COMPL = ''' + @dtLogged + ''',
NOTES = ''' + @strRemarks + ''',
LAST_UPDT_NM = ''' + @strLogged + ''',
LAST_UPDT_DT = '''+ @Datenow +''''
exec(@execsql)
Print @Personnel
Print @execsql
END
FETCH Next From MPDV INTO @QId,@MPDV,@answer,@dtLogged,@strRemarks,@strLogged
Close MPDV
Deallocate MPDV
end
FETCH Next From Pers INTO @Personnel
Close Pers
deallocate Pers
August 17, 2011 at 9:51 am
You should move your second
FETCH Next From MPDV INTO @QId,@MPDV,@answer,@dtLogged,@strRemarks,@strLogged
inside of loop
The same is for
FETCH Next From Pers INTO @Personnel
I guess it's cut-&-paste one 😉
August 17, 2011 at 9:58 am
Hi, do you mean like this?? I seems it might be updating twice but this also might be the way thewquestions where designed because I look at what pulls and its only sigle records.
Declare Pers Cursor For
Select Distinct intpersonnelId from tblSrpEventDataHistory where intSrpAttendId in
(select intSrpAttendId from tblSRPAttendance where intEventId = @EventId) order by intPersonnelID
OPEN Pers
FETCH Next From Pers INTO @Personnel
WHILE @@FETCH_STATUS = 0
begin
Declare MPDV Cursor FOR
Selected.intquestionId,
q.intMPDVID,
cast(Case When ed.intAnswer = 0 THEN 'GO' WHEN ed.intAnswer = 1 THEN 'NO GO' ELSE 'NO GO' END as varchar(6)),
ed.dtLogged,
ed.strRemarks,
ed.strLogged
FromtblSRPEventDataHistory as ed INNER JOIN
tblSRPQuestion as q on q.intQuestionId = ed.intQuestionId
Whereq.intMPDVID IS NOT NULL AND intPersonnelID = @Personnel
Order by intAnswer
OPEN MPDV
FETCH Next From MPDV INTO @QId,@MPDV,@answer,@dtLogged,@strRemarks,@strLogged
WHILE @@FETCH_STATUS = 0
BEGIN
set @execsql = 'update OPENQUERY(RCASDBOR, ''Select STA, DT_TM_COMPL, NOTES, LAST_UPDT_NM, LAST_UPDT_DT FROM DOIM_RO2.CHECKLIST
WHERETASK_CTRL_SEQ_ID = '+ @MPDV + ' AND
UNIT_SOLDIER_ID = '+ @Personnel +''')
SETSTA = ''' + @answer + ''',
DT_TM_COMPL = ''' + @dtLogged + ''',
NOTES = ''' + @strRemarks + ''',
LAST_UPDT_NM = ''' + @strLogged + ''',
LAST_UPDT_DT = '''+ @Datenow +''''
exec(@execsql)
Print @Personnel
Print @execsql
FETCH Next From MPDV INTO @QId,@MPDV,@answer,@dtLogged,@strRemarks,@strLogged
END
Close MPDV
Deallocate MPDV
FETCH Next From Pers INTO @Personnel
end
Close Pers
deallocate Pers
August 17, 2011 at 10:16 am
Yes like this:
DECLARE name CURSOR ...
FOR SELECT ....
OPEN name
FETCH NEXT FROM name INTO @var
WHILE @@FETCH_STATUS = 0
BEGIN
... Do some work
FETCH NEXT FROM name INTO @var
END
CLOSE name
DEALLOCATE name
August 17, 2011 at 10:32 am
Using a better pattern for your cursor loops you would have easily spotted this one. I always write my cursor loops (when I do write one ;-)) like this:
declare cur cursor local fast_forward
for
select t.col
from dbo.tbl t;
open cur;
while 1 = 1
begin
declare @col int;
fetch next from cur into @col;
if @@fetch_status = -1
break;
if @@fetch_status = 0
begin
-- Do whatever you need done here.
end
end
close cur;
deallocate cur;
This way you need per loop only 1 call to fetch into instead of 2. This eliminates the risk for the most commonly made error with cursors: forgetting to put the proper variable(s) in the 2nd fetch into call. Plus it makes very clear what initiates the loop, what finalizes it and what is done inside the loop. You would have easily spotted the missing finalization at the end of your code: close Pers and deallocate Pers. But best for this case: You could not possibly have made the same error; as the first fetch is the only one and this is always repeated.
Once you're used to this pattern, you'll never want to go back anymore.
August 17, 2011 at 11:40 am
HI,
I am always game for improvement, so this is what I came up with but its seems to run about 5 mintues longer than the fix I applied to the one above so I guess i am checking to make sure I did it correctly.
declare @Personnel varchar(9), @execsql nvarchar (4000), @answer varchar(6), @dtlogged varchar(30), @strRemarks varchar(1000), @strLogged varchar(100),
@MPDV varchar(10), @QId int, @Datenow as varchar(35)
set @Datenow = cast(GETDATE() as varchar(max))
Declare MPDV Cursor FOR
Selected.intquestionId,
q.intMPDVID,
cast(Case When ed.intAnswer = 0 THEN 'GO' WHEN ed.intAnswer = 1 THEN 'NO GO' ELSE 'NO GO' END as varchar(6)),
ed.dtLogged,
ed.strRemarks,
ed.strLogged,
intPersonnelID
FromtblSRPEventDataHistory as ed INNER JOIN
tblSRPQuestion as q on q.intQuestionId = ed.intQuestionId
Whereq.intMPDVID IS NOT NULL AND intPersonnelID IN ( Select Distinct intpersonnelId from tblSrpEventDataHistory where intSrpAttendId in
(select intSrpAttendId from tblSRPAttendance where intEventId = 146)) order by intPersonnelID, intAnswer
OPEN MPDV;
While 1=1
BEGIN
FETCH Next From MPDV INTO @QId,@MPDV,@answer,@dtLogged,@strRemarks,@strLogged,@Personnel;
IF @@FETCH_STATUS = -1
break;
If @@FETCH_STATUS = 0
BEGIN
set @execsql = 'update OPENQUERY(RCASDBOR, ''Select STA, DT_TM_COMPL, NOTES, LAST_UPDT_NM, LAST_UPDT_DT FROM DOIM_RO2.CHECKLIST
WHERETASK_CTRL_SEQ_ID = '+ @MPDV + ' AND
UNIT_SOLDIER_ID = '+ @Personnel +''')
SETSTA = ''' + @answer + ''',
DT_TM_COMPL = ''' + @dtLogged + ''',
NOTES = ''' + @strRemarks + ''',
LAST_UPDT_NM = ''' + @strLogged + ''',
LAST_UPDT_DT = '''+ @Datenow +''''
exec(@execsql)
End
End
end
Close MPDV;
Deallocate MPDV;
August 17, 2011 at 12:50 pm
The time difference you notice will be from the fact that the query on your new version is completely different from that on the first version. If you don't change the queries there will normally not be any performance difference between the two different patterns.
Viewing 7 posts - 1 through 6 (of 6 total)
You must be logged in to reply to this topic. Login to reply