Cursor Not Working As Expected

Question

Cursor Not Working As Expected

donne4real

Old Hand

Points: 386
More actions
January 6, 2010 at 11:59 am

#214173

Here is my code but for some reason, it seems not to be working correctly. It is not going through the loop as expected and will appreciate it if you guys can help me with it. I added some print statements within the code to be able to see where it is getting stuck and it seems to be within the loop where the issue is.
BEGIN
declare @Cmd varchar(255)
declare @filename varchar(255)
declare @expectedfilename varchar(255)
declare @batfilename varchar(255)
declare @sys_error int
declare @outputcount int
declare DailyMail_Cursor cursor local fast_forward
for
select filename from dailymail
open DailyMail_Cursor
fetch next from DailyMail_Cursor into @filename
while @@fetch_status = 0
begin
select @expectedfilename = expectedfilename from FILEDIRECTORY
select @batfilename = batfilename from FILEDIRECTORY where @expectedfilename =@filename
print 'test'
select @outputcount = count(*) from filedirectory where @expectedfilename =@filename
print @outputcount
if @outputcount <>0
--if @expectedfilename = @filename
print 'after if stmt'
begin
print 'after the inner begin stmt'
select @Cmd = 'C:\test\' + @batfilename
print @cmd
EXECUTE @sys_error = master.dbo.xp_cmdshell @cmd--, no_output
end
fetch next from DailyMail_Cursor into @filename
end
print 'after begin'
--
END
close DailyMail_Cursor
deallocate DailyMail_Cursor

Viewing 9 posts - 1 through 8 (of 8 total)

You must be logged in to reply to this topic. Login to reply

SQLRNNR SSC Guru Points: 281334 More actions · Answer 1

donne4real (1/6/2010)
Here is my code but for some reason, it seems not to be working correctly. It is not going through the loop as expected and will appreciate it if you guys can help me with it. I added some print statements within the code to be able to see where it is getting stuck and it seems to be within the loop where the issue is.
BEGIN
declare @Cmd varchar(255)
declare @filename varchar(255)
declare @expectedfilename varchar(255)
declare @batfilename varchar(255)
declare @sys_error int
declare @outputcount int
declare DailyMail_Cursor cursor local fast_forward
for
select filename from dailymail
open DailyMail_Cursor
fetch next from DailyMail_Cursor into @filename
while @@fetch_status = 0
begin
select @expectedfilename = expectedfilename from FILEDIRECTORY
select @batfilename = batfilename from FILEDIRECTORY where @expectedfilename =@filename
print 'test'
select @outputcount = count(*) from filedirectory where @expectedfilename =@filename
print @outputcount
if @outputcount <>0
--if @expectedfilename = @filename
print 'after if stmt'
begin
print 'after the inner begin stmt'
select @Cmd = 'C:\test\' + @batfilename
print @cmd
EXECUTE @sys_error = master.dbo.xp_cmdshell @cmd--, no_output
end
fetch next from DailyMail_Cursor into @filename
end
print 'after begin'
--
END
close DailyMail_Cursor
deallocate DailyMail_Cursor

I recommend changing this to a set based method.

You are missing a ' at the end of your select @Cmd statement.

Jason...AKA CirqueDeSQLeil
_______________________________________________
I have given a name to my pain...MCM SQL Server, MVP
SQL RNNR
Posting Performance Based Questions - Gail Shaw[/url]
Learn Extended Events

donne4real Old Hand Points: 386 More actions · Answer 2

I think I should have added more information. I have 2 tables. It loops through the table to see if a filename exists in both. And if it does, calls the specified batch file (@batfilename). There are different batch files for the different filenames.

Jeffrey Williams SSC Guru Points: 90244 More actions · Answer 3

donne4real (1/6/2010)
I think I should have added more information. I have 2 tables. It loops through the table to see if a filename exists in both. And if it does, calls the specified batch file (@batfilename). There are different batch files for the different filenames.

It really depends on what those batch files are doing - but I wouldn't use SQL Server to control this process. I would look at controlling the process from the OS level - and just using calls to SQL Server to get the data from the tables that is needed.

Powershell would be ideal for this - as well as using SQLCMD and would not require opening up access to xp_cmdshell to process the batch files.

Jeffrey Williams
“We are all faced with a series of great opportunities brilliantly disguised as impossible situations.”

― Charles R. Swindoll

How to post questions to get better answers faster
Managing Transaction Logs

AndrewSQLDBA SSC-Insane Points: 21827 More actions · Answer 4

I strongly agree with Jason, and stop using a Cursor.

They are only a curse on a database.

Andrew SQLDBA

donne4real Old Hand Points: 386 More actions · Answer 5

I recommend changing this to a set based method.
You are missing a ' at the end of your select @Cmd statement.

Jason, please can you elaborate what you mean by the set based method? I am a newbie and I am still learning.

SQLRNNR SSC Guru Points: 281334 More actions · Answer 6

Jeff Moden has written some good articles here at SSC about triangular joins, set based method, and RBAR. I think those articles would be better able to fully explain it.

In short, set-based queries perform an action against a group of records (1 or more records) at the same time. Cursors and loops (RBAR) perform an action to one record at a time. This kind of processing can be intensive on resources and cause other issues.

My sig has one such article by Jeff. Check it out.

Jason...AKA CirqueDeSQLeil
_______________________________________________
I have given a name to my pain...MCM SQL Server, MVP
SQL RNNR
Posting Performance Based Questions - Gail Shaw[/url]
Learn Extended Events

Jeff Moden SSC Guru Points: 1003851 More actions · Answer 7

As a side bar for everyone else... a cursor isn't such a bad thing for such a control, here. You could concatenate all of the commands together in one big string, but it won't buy you much for performance because the "loop" isn't the slow thing in this case.

--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)

Jeff Moden SSC Guru Points: 1003851 More actions · Answer 8

donne4real (1/6/2010)
I think I should have added more information. I have 2 tables. It loops through the table to see if a filename exists in both. And if it does, calls the specified batch file (@batfilename). There are different batch files for the different filenames.

Apparently it doesn't because you said it get's stuck... of course, we have no idea what you mean by that. Can you be a wee bit more descriptive?

Since you're new here, you might want to take a look at the first link in my signature below... makes people respond to your problems quicker provided that you give more of a description than "it's stuck". 😉

--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)