October 1, 2015 at 1:10 pm
Hi folks,
I have used cursors in a similar way 100 times (but not in a couple years). I am selecting 4 columns from a table and inserting them into 4 variables and then calling a stored proc with those 4 and a few other already defined variables. The already set variables are fine. But the variables which should be set by the cursor are coming up as NULL (I can tell because they are spit out as 'NULL' by the RAISERROR). There are no rows with NULL values in the table...
I'm sure it's something simple / stupid but I can't see it. Can someone help?
Thanks!!
BEGIN TRY
-- Set CR First Last for each report type
DECLARE@RowNumOrderBy VARCHAR(40),
@StatCode VARCHAR(50),
@UpdateTargetField VARCHAR(50),
@UpdateSourceField VARCHAR(50)
--cursor through Settings_FirstLast table and run dyn sql with settings from each row
DECLARE SetFirstLast CURSOR
FOR
SELECTRowNumOrderBy, StatCode, UpdateTargetField, UpdateSourceField
FROMSettings_FirstLast
OPEN SetFirstLast
FETCH NEXT FROM SetFirstLast INTO @RowNumOrderBy, @StatCode, @UpdateTargetField, @UpdateSourceField
WHILE @@FETCH_STATUS = 0
BEGIN
EXEC DynUpdate_CRFirstLast @RowNumOrderBy,
@EffMonthBegin,
@EffMonthEnd,
@StatCode,
@UpdateTargetField,
@UpdateSourceField,
@NoStatusBeforeDate,
@NoStatusAfterDate
FETCH NEXT FROM SetFirstLast
INTO @RowNumOrderBy, @StatCode, @UpdateTargetField, @UpdateSourceField
END
CLOSE SetFirstLast
DEALLOCATE SetFirstLast
END TRY
BEGIN CATCH
SELECT @msg = 'ERROR calling DynUpdate_CRFirstLast. @RowNumOrderBy = ' + isnull(@RowNumOrderBy,'NULL') +
' @EffMonthBegin = ' + isnull(convert(varchar,@EffMonthBegin,101),'NULL') + ' @EffMonthEnd = ' + isnull(convert(varchar,@EffMonthEnd,101),'NULL') +
' @StatCode = ' + isnull(@StatCode,'NULL') + ' @UpdateTargetField = ' + isnull(@UpdateTargetField,'NULL') + ' @UpdateSourceField = ' + isnull(@UpdateSourceField,'NULL') +
' @NoStatusBeforeDate = ' + isnull(convert(varchar,@NoStatusBeforeDate,101),'NULL') + ' @NoStatusAfterDate = ' + isnull(convert(varchar,@NoStatusAfterDate,101),'NULL');
RAISERROR (@msg,16,1);
RETURN 1;
END CATCH
October 1, 2015 at 1:26 pm
agerard (10/1/2015)
Hi folks,I have used cursors in a similar way 100 times (but not in a couple years). I am selecting 4 columns from a table and inserting them into 4 variables and then calling a stored proc with those 4 and a few other already defined variables. The already set variables are fine. But the variables which should be set by the cursor are coming up as NULL (I can tell because they are spit out as 'NULL' by the RAISERROR). There are no rows with NULL values in the table...
I'm sure it's something simple / stupid but I can't see it. Can someone help?
Thanks!!
BEGIN TRY
-- Set CR First Last for each report type
DECLARE@RowNumOrderBy VARCHAR(40),
@StatCode VARCHAR(50),
@UpdateTargetField VARCHAR(50),
@UpdateSourceField VARCHAR(50)
--cursor through Settings_FirstLast table and run dyn sql with settings from each row
DECLARE SetFirstLast CURSOR
FOR
SELECTRowNumOrderBy, StatCode, UpdateTargetField, UpdateSourceField
FROMSettings_FirstLast
OPEN SetFirstLast
FETCH NEXT FROM SetFirstLast INTO @RowNumOrderBy, @StatCode, @UpdateTargetField, @UpdateSourceField
WHILE @@FETCH_STATUS = 0
BEGIN
EXEC DynUpdate_CRFirstLast @RowNumOrderBy,
@EffMonthBegin,
@EffMonthEnd,
@StatCode,
@UpdateTargetField,
@UpdateSourceField,
@NoStatusBeforeDate,
@NoStatusAfterDate
FETCH NEXT FROM SetFirstLast
INTO @RowNumOrderBy, @StatCode, @UpdateTargetField, @UpdateSourceField
END
CLOSE SetFirstLast
DEALLOCATE SetFirstLast
END TRY
BEGIN CATCH
SELECT @msg = 'ERROR calling DynUpdate_CRFirstLast. @RowNumOrderBy = ' + isnull(@RowNumOrderBy,'NULL') +
' @EffMonthBegin = ' + isnull(convert(varchar,@EffMonthBegin,101),'NULL') + ' @EffMonthEnd = ' + isnull(convert(varchar,@EffMonthEnd,101),'NULL') +
' @StatCode = ' + isnull(@StatCode,'NULL') + ' @UpdateTargetField = ' + isnull(@UpdateTargetField,'NULL') + ' @UpdateSourceField = ' + isnull(@UpdateSourceField,'NULL') +
' @NoStatusBeforeDate = ' + isnull(convert(varchar,@NoStatusBeforeDate,101),'NULL') + ' @NoStatusAfterDate = ' + isnull(convert(varchar,@NoStatusAfterDate,101),'NULL');
RAISERROR (@msg,16,1);
RETURN 1;
END CATCH
Maybe instead of fixing the cursor you can remove it? Maybe you can edit the procedure so it can receive a table valued parameter instead of scalar values?
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
October 1, 2015 at 1:34 pm
Sure, I could use something else other than a cursor. maybe that'd fix the issue. But... in this case the dataset i am looping through is small so a cursor is appropriate. I am hoping that I just did something wrong and someone could point it out to me. because it really doesn't make sense.
October 1, 2015 at 1:52 pm
tweaked the catch block to return the error message (duh), but now i'm even more confused:
ERROR_MESSAGE = A cursor with the name 'SetFirstLast' already exists.
There is no other looping going on...
October 1, 2015 at 1:58 pm
agerard (10/1/2015)
Sure, I could use something else other than a cursor. maybe that'd fix the issue. But... in this case the dataset i am looping through is small so a cursor is appropriate. I am hoping that I just did something wrong and someone could point it out to me. because it really doesn't make sense.
I would argue that a cursor is not appropriate regardless of the dataset size. I would say that a cursor is the wrong tool for the job because it is slow. In this case, you don't notice that is the wrong tool for the job because the small dataset doesn't have a huge impact on performance.
What happens in the future when the dataset is larger? Or when somebody else sees your code and uses it as an example?
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
October 1, 2015 at 2:02 pm
agerard (10/1/2015)
tweaked the catch block to return the error message (duh), but now i'm even more confused:ERROR_MESSAGE = A cursor with the name 'SetFirstLast' already exists.
There is no other looping going on...
My guess is that you had an error in your code at one point and your catch block caught the error. And since you have the drop inside the TRY it never dropped it. You should move the declare outside the TRY/CATCH block and then move the close / deallocate after the CATCH. The way you have it now if an error is thrown the cursor doesn't get deallocated. Select just the close/deallocate lines and run them by themselves. I suspect you will get "command(s) completed".
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
October 1, 2015 at 2:11 pm
The dataset will never be large. And "slow" is all relative. I know cursors get a bad rap but defending them is not really why I posted...
October 1, 2015 at 2:12 pm
re: moving the declare / deallocate outside of the try. That might help, thanks. In the mean time I ditched the cursor for another method and it seems to be working now. :rolleyes:
October 1, 2015 at 2:19 pm
You can check to see if the cursor exists before you DECLARE it. I added "LOCAL" to the cursor declaration so that you know for sure that it's local and not global. I added "FAST_FORWARD" to make the cursor more efficient since you're only using FETCH NEXT.
BEGIN TRY
-- Set CR First Last for each report type
DECLARE@RowNumOrderBy VARCHAR(40),
@StatCode VARCHAR(50),
@UpdateTargetField VARCHAR(50),
@UpdateSourceField VARCHAR(50)
IF CURSOR_STATUS ('LOCAL', 'SetFirstLast') >= -1
BEGIN
DEALLOCATE SetFirstLast
END --IF
--cursor through Settings_FirstLast table and run dyn sql with settings from each row
DECLARE SetFirstLast CURSOR LOCAL FAST_FORWARD
FOR
SELECTRowNumOrderBy, StatCode, UpdateTargetField, UpdateSourceField
FROMSettings_FirstLast
OPEN SetFirstLast
FETCH NEXT FROM SetFirstLast INTO @RowNumOrderBy, @StatCode, @UpdateTargetField, @UpdateSourceField
WHILE @@FETCH_STATUS = 0
BEGIN
EXEC DynUpdate_CRFirstLast @RowNumOrderBy,
@EffMonthBegin,
@EffMonthEnd,
@StatCode,
@UpdateTargetField,
@UpdateSourceField,
@NoStatusBeforeDate,
@NoStatusAfterDate
FETCH NEXT FROM SetFirstLast
INTO @RowNumOrderBy, @StatCode, @UpdateTargetField, @UpdateSourceField
END
CLOSE SetFirstLast
DEALLOCATE SetFirstLast
END TRY
BEGIN CATCH
...
SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.
October 1, 2015 at 2:19 pm
agerard (10/1/2015)
re: moving the declare / deallocate outside of the try. That might help, thanks. In the mean time I ditched the cursor for another method and it seems to be working now. :rolleyes:
Cool. Glad you found a solution that isn't cursor based.
The reason moving the cursor stuff outside is because it would fall to the catch and never hit the line to close your cursor the way you had it.
The argument/discussion about cursors has happened a million times. There is almost always a set based solution so you should always strive to find them.
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
October 1, 2015 at 2:19 pm
Looks like there was a problem in the stored proc being called inside of the cursor, and as you suggested the whole thing being inside of the try catch was causing the error message to be deceptive.
Thanks
Viewing 11 posts - 1 through 10 (of 10 total)
You must be logged in to reply to this topic. Login to reply