June 2, 2015 at 3:10 pm
Hugo Kornelis (6/2/2015)
Alan.B (6/1/2015)
Until someone, anyone, can post an example where a cursor or other iterative method was faster than a well-written set-based solution I will remain convinced that the set-based solution is the way to go.A few years ago, I published a set of blog posts about the bin packing problem. I presented a few cursor-based solutions, a fully set-based solution, and then my own hybrid "iteration + set-based" solution. The fully set-based solution was far slower then the cursor-based solutions. This was before windowing functions were introduced so I cannot 100% certain say that this is still correct, but I personally do not know of any way to employ these to create a better set-based bin packing query.
Link: http://sqlblog.com/blogs/hugo_kornelis/archive/tags/Bin+Packing/default.aspx
Note that this series of blog posts was inspired by a real world problem (not from my own clients, but from a question I answered on internet).
I need to do variations of bin packing and read your blog posts some time back, they come up first when I do a search for (good stuff). I noticed that in one of the threads, Dwain Camps said that he had a purely set-based way of handling bin packing that performed well but cannot find an article about that. I plan on revisiting your article and playing around with solutions using Window and 2012 framing functions.
-- Itzik Ben-Gan 2001
June 2, 2015 at 3:12 pm
fundpc (6/2/2015)
...and yes I am old enough to have been there when our IBM RD281 disk pack, read heads driven with hydraulic rams, blew a packing and saturated the innards with hydraulic fluid. YUCK...
Old school memory leak! :w00t:
June 2, 2015 at 3:25 pm
Andy DBA (6/2/2015)
fundpc (6/2/2015)
...and yes I am old enough to have been there when our IBM RD281 disk pack, read heads driven with hydraulic rams, blew a packing and saturated the innards with hydraulic fluid. YUCK...Old school memory leak! :w00t:
Nice!
-- Itzik Ben-Gan 2001
June 3, 2015 at 2:55 am
There are people who think that avoiding cursor doesn't mean to avoid row by row processing, so they are going to substitute cursor with while loops. I guess that microsft is to blame for that misunderstanding and that Always set operations should be preferred more then anything else even if sometimes are somewhat more cryptic. So totally agree with this topic.
June 3, 2015 at 5:34 am
Regarding the GOTO statement, are people saying it has no place in TSQL either?
I know VB and remember on my Amstrad 64 having to use GOTO statements constantly so in code yeah I can understand the sentiment but in a stored proc with an error handler is anyone saying the GOTO statement should be redundant?
Here is an example stored proc with some GOTO statements to control basic error handling and also use an ext sections.
This is a VERY SIMPLIFIED version of the stored proc with missing chunks of Insert/Update statements I have just put comments in
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER PROCEDURE [dbo].[usp_asp_save_job]
@SitePK int = 0,
@ClientPK int = 0,
@JobPK int = 0, -- 0 = NEW JOB, > 0 EDIT JOb
@JobTitle nvarchar(255) = NULL
@JobDescription nvarchar(max) = NULL,
@ApplicationFolders bit = 0
AS
SET NOCOUNT ON
SET DATEFORMAT YMD
DECLARE @success bit,
@message nvarchar(255),
@errors int,
@rowcount int,
@ChangeCode tinyint,
@Action nvarchar(100)
-- ensure we have some KEY data - application does real validation this is just a double check to ensure direct calls to the proc don't insert blank jobs
IF ISNULL(@JobTitle,'') != '' OR ISNULL(@JobDescription,'') != '' OR @SitePK = 0 OR @ClientPK = 0
BEGIN
-- get appropriate error message for this site
SELECT @message= dbo.udf_GET_MESSAGE(@SitePK, 700) -- e.g "no data supplied "
-- jump to the error handler
GOTO HANDLE_ERROR
END
-- check for existing job posted with same details within last 60 millsecond e.g double post
IF EXISTS(SELECT jobPK FROM JOBS WHERE sitefk = @sitepk and ClientFK = @ClientPK and jobTitle = @JobTitle and jobDescription = @JobDescription AND DATEDDIFF(mi,livedate,getdate())<60)
BEGIN
-- get appropriate error message for this error from list of messages using a function and the code for the error 500 - message could be in any lanugage so pass sitePK to get relevant site message
SELECT @message= dbo.udf_GET_MESSAGE(@SitePK, 500)
-- jump to error handler
GOTO HANDLE_ERROR
END
-- before updating work out what has changed for job history log table
SELECT@Details = dbo.udf_GET_JOB_CHANGE_DETAILS(@SitePK, @JobPK)
-- Do update as we have jobID
IF @JobPk != 0
BEGIN
-- Get Edit Job message for site in appropriate language / wording for site
SELECT @ChangeCode = 2 ,@Action = dbo.udf_GET_MESSAGE(@SitePK, 456)
BEGIN TRAN
UPDATEJOBS
SETClientFK= @ClientPK,
JobTitle= @JobTitle,
JobDescription= @JobDescription
-- others of course in real proc
WHEREJobPK = @JobPK
SELECT @errors = @@error , @rowcount = @@rowcount
IF @errors != 0 OR @rowcount <> 1
BEGIN
-- get correct update error message
SELECT @message = dbo.udf_GET_ERROR_MESSAGE(@SitePK, 101)
-- go to error handler
GOTO HANDLE_ERROR
END
-- if application folders are enabled for the site update group name in case it's changed (these are folders people can put jobs in e.g "best jobs", "IT jobs"
IF @ApplicationFolders = 1
BEGIN
-- check if group already exists and if it does ensure the group name is updated to the job title in case its changed
IF EXISTS(SELECT GroupPK FROM GROUPS WHERE clientFK = @ClientPK AND JobFK = @JobPK)
BEGIN
UPDATEGROUPS
SETGroupName = @JobTitle
WHEREClientFK = @ClientPK AND
JobFK = @JobPK
END
ELSE
BEGIN
-- create new group for all applications to this job to go into by default
INSERT INTO GROUPS
(GroupName, ClientFK, GroupType, JobFK)
VALUES
(@JobTitle, @ClientPk, 'DJA', @JobPK)
END
-- ensure no errors and set message if there was a group error
SELECT @errors = @@error , @rowcount = @@rowcount
IF @errors != 0 OR @rowcount <> 1
BEGIN
-- get correct group inset error message
SELECT @message = dbo.udf_GET_ERROR_MESSAGE(@SitePK, 220)
-- go to error handler
GOTO HANDLE_ERROR
END
END
END
ELSE
BEGIN
-- Get Add Job message for site in appropriate language / wording for site
SELECT @ChangeCode = 1 , @Action = dbo.udf_GET_MESSAGE(@SitePK, 455)
BEGIN TRAN
-- short example of a new job
INSERT INTO JOBS
(SiteFK, ClientFK,JobTitle,JobDescription)
VALUES
(@SitePK, @ClientPK, @JobTitle, @JobDescription)
-- get new jobID
SELECT @errors = @@error, @rowcount = @@rowcount, @JobPK = SCOPE_IDENTITY()
-- if error
IF @errors != 0 OR @rowcount<>1
BEGIN
-- get correct update error message
SELECT @message = dbo.udf_GET_ERROR_MESSAGE(@SitePK, 102)
-- go to error handler
GOTO HANDLE_ERROR
END
-- if application folders are on for the site create a new group
IF @ApplicationFolders = 1
BEGIN
-- ensure group is clean without a check as that just wastes time IF it does exist already, obviously if it doesn't then it won't do anthing but the fields are indexed
DELETE
FROMGROUPS
WHEREJobFK = @JobPK
AND GroupType = 'DJA'
AND ClientFK = @ClientPK
-- doesn't matter if nothing was deleted
INSERT INTO GROUPS
(GroupName, ClientFK, GroupType, JobFK)
VALUES
(@JobTitle, @ClientPk, 'DJA', @JobPK)
-- ensure no errors and set message if there was a group error
SELECT @errors = @@error, @rowcount = @@rowcount
IF @errors != 0 OR @rowcount <> 1
BEGIN
-- get correct group inset error message
SELECT @message = dbo.udf_GET_ERROR_MESSAGE(@SitePK, 220)
-- go to error handler
GOTO HANDLE_ERROR
END
END
END
-- other work related to multiple linked sites
-- other code related to handle PENDING (non live) jobs
-- if we get here NO ERRORS so COMMIT the transaction
IF @@TRANCOUNT = 0
COMMIT TRAN
-- set return values including a success message and the return code for the application
SELECT @success = 1, @message = dbo.udf_GET_MESSAGE(@SitePK, 103)
-- skip over error handler and exit proc
GOTO EXIT_PROC
END
-- error handler code - cannot be in here if NO ERROR which is why GOTO jumps OVER it
HANDLE_ERROR:
-- if we have an open transaction close it
IF @@TRANCOUNT > 0
ROLLBACK TRAN
-- if we don't have an error message get a default on e.g "Job failed to save please try back later"
IF ISNULL(@message,'') = ''
SELECT @message = dbo.udf_GET_ERROR_MESSAGE(@SitePK, 100)
-- we failed so succes is 0
SELECT @success = 0
-- exit the stored proc
EXIT_PROC:
-- before we exit the proc we log any changes to the table and any error messages if thee were any
IF INSULL(@Details,'') != ''
BEGIN
EXEC [dbo].[usp_sql_add_job_change_record] @SitePK, @JobPK, @ChangeCode, @Action, @Details, @Success, @Message
END
-- return to caller
SELECT cast(@Success as int) as Success, isnull(@JobPK,0) as JobPK, @message as [message], @CheckMode as checkmode
-- use success as return code in case we are using an ADO stored proce that relies on return codes
RETURN @Success
Also if I had a stored proc that saved MULTIPLE jobs at the same time e.g @JobID was not ONE integer but a comma delimited list of integers to save in one go, how could I not use a cursor to loop through them all when for EACH JOB I have statements to check for duplicate submits, finding out what has changed in the job for reporting, plus logging that information with another stored proc call that is used across the site for logging info regarding changes to important records.
Cursors have their time and place especially if you have to do procedural type procedure like the one above. I have seen people just try and replace cursors with temporary tables with identity fields and then WHILE loops selecting the next ID record before doing the middle content of a proc like the one above before incrementing the loop counter. This is just creating a cursor in another manner and if you have to do a procedural type proc then a FAST_FORWARD LOCAL CURSOR is designed for looping from start to end so why try and replace it.
If anyone can show me how I can replace all the contents of the proc above without GOTO statements and why the format used is bad I would like to know.
Also if you can imagine having multiple jobs to handle in the proc rather than one, so @JobIDs varchar(1000) (e.g 12,13,14), @JobTitle nvarchar(max) 'Test 1','Test 2','Test 3' (Same for Job Desc) e.g position N of each comma delimited list relates to position N of the other values I would be interested to see.
Each job needs the details of the changes done to it logged, the related groups checked and inserted/updated, plus the changes all logged with the stored proc currently residing in the EXIT_PROC part of the proc. In a cursor loop this would be at the bottom of the loop iteration.
And I don't use cursors if I can do it in a set based by the way.
Just when the data logic is more complicated than a single SELECT/UPDATE/INSERT statement and requires calls to other functions / procedures.
Thanks
Rob
June 3, 2015 at 5:47 am
Lenochka (6/1/2015)
Thank you for writing this. I used to ask a cursor question when interviewing candidates and I got the same answer every time “you shouldn’t use cursors, they are bad”. I found it to be extremely frustrating. Developers use that as an excuse to never learn how to write cursors or when to use them. I have seen people write very un-maintainable, convoluted code (that might not always work correctly), or resort to manual processing in places where a cursor is an easy, clean an elegant solution.
Cursors don't kill performance.
People (using cursors inappropriately) kill performance.
June 3, 2015 at 5:53 am
Andy DBA (6/2/2015)
fundpc (6/2/2015)
...and yes I am old enough to have been there when our IBM RD281 disk pack, read heads driven with hydraulic rams, blew a packing and saturated the innards with hydraulic fluid. YUCK...Old school memory leak! :w00t:
BWAAAA-HAAAA!!!!! Way too funny! 😛
--Jeff Moden
Change is inevitable... Change for the better is not.
June 3, 2015 at 5:56 am
Lenochka (6/1/2015)
Thank you for writing this. I used to ask a cursor question when interviewing candidates and I got the same answer every time “you shouldn’t use cursors, they are bad”. I found it to be extremely frustrating. Developers use that as an excuse to never learn how to write cursors or when to use them. I have seen people write very un-maintainable, convoluted code (that might not always work correctly), or resort to manual processing in places where a cursor is an easy, clean an elegant solution.
Not being combative, here. I'd really like to know. Can you explain what some of those "elegant" solutions that actually required a cursor were?
--Jeff Moden
Change is inevitable... Change for the better is not.
June 3, 2015 at 6:34 am
Rob Reid-246754 (6/3/2015)
Regarding the GOTO statement, are people saying it has no place in TSQL either?I know VB and remember on my Amstrad 64 having to use GOTO statements constantly so in code yeah I can understand the sentiment but in a stored proc with an error handler is anyone saying the GOTO statement should be redundant?
Here is an example stored proc with some GOTO statements to control basic error handling and also use an ext sections.
This is a VERY SIMPLIFIED version of the stored proc with missing chunks of Insert/Update statements I have just put comments in
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER PROCEDURE [dbo].[usp_asp_save_job]
@SitePK int = 0,
@ClientPK int = 0,
@JobPK int = 0, -- 0 = NEW JOB, > 0 EDIT JOb
@JobTitle nvarchar(255) = NULL
@JobDescription nvarchar(max) = NULL,
@ApplicationFolders bit = 0
AS
SET NOCOUNT ON
SET DATEFORMAT YMD
DECLARE @success bit,
@message nvarchar(255),
@errors int,
@rowcount int,
@ChangeCode tinyint,
@Action nvarchar(100)
-- ensure we have some KEY data - application does real validation this is just a double check to ensure direct calls to the proc don't insert blank jobs
IF ISNULL(@JobTitle,'') != '' OR ISNULL(@JobDescription,'') != '' OR @SitePK = 0 OR @ClientPK = 0
BEGIN
-- get appropriate error message for this site
SELECT @message= dbo.udf_GET_MESSAGE(@SitePK, 700) -- e.g "no data supplied "
-- jump to the error handler
GOTO HANDLE_ERROR
END
-- check for existing job posted with same details within last 60 millsecond e.g double post
IF EXISTS(SELECT jobPK FROM JOBS WHERE sitefk = @sitepk and ClientFK = @ClientPK and jobTitle = @JobTitle and jobDescription = @JobDescription AND DATEDDIFF(mi,livedate,getdate())<60)
BEGIN
-- get appropriate error message for this error from list of messages using a function and the code for the error 500 - message could be in any lanugage so pass sitePK to get relevant site message
SELECT @message= dbo.udf_GET_MESSAGE(@SitePK, 500)
-- jump to error handler
GOTO HANDLE_ERROR
END
-- before updating work out what has changed for job history log table
SELECT@Details = dbo.udf_GET_JOB_CHANGE_DETAILS(@SitePK, @JobPK)
-- Do update as we have jobID
IF @JobPk != 0
BEGIN
-- Get Edit Job message for site in appropriate language / wording for site
SELECT @ChangeCode = 2 ,@Action = dbo.udf_GET_MESSAGE(@SitePK, 456)
BEGIN TRAN
UPDATEJOBS
SETClientFK= @ClientPK,
JobTitle= @JobTitle,
JobDescription= @JobDescription
-- others of course in real proc
WHEREJobPK = @JobPK
SELECT @errors = @@error , @rowcount = @@rowcount
IF @errors != 0 OR @rowcount <> 1
BEGIN
-- get correct update error message
SELECT @message = dbo.udf_GET_ERROR_MESSAGE(@SitePK, 101)
-- go to error handler
GOTO HANDLE_ERROR
END
-- if application folders are enabled for the site update group name in case it's changed (these are folders people can put jobs in e.g "best jobs", "IT jobs"
IF @ApplicationFolders = 1
BEGIN
-- check if group already exists and if it does ensure the group name is updated to the job title in case its changed
IF EXISTS(SELECT GroupPK FROM GROUPS WHERE clientFK = @ClientPK AND JobFK = @JobPK)
BEGIN
UPDATEGROUPS
SETGroupName = @JobTitle
WHEREClientFK = @ClientPK AND
JobFK = @JobPK
END
ELSE
BEGIN
-- create new group for all applications to this job to go into by default
INSERT INTO GROUPS
(GroupName, ClientFK, GroupType, JobFK)
VALUES
(@JobTitle, @ClientPk, 'DJA', @JobPK)
END
-- ensure no errors and set message if there was a group error
SELECT @errors = @@error , @rowcount = @@rowcount
IF @errors != 0 OR @rowcount <> 1
BEGIN
-- get correct group inset error message
SELECT @message = dbo.udf_GET_ERROR_MESSAGE(@SitePK, 220)
-- go to error handler
GOTO HANDLE_ERROR
END
END
END
ELSE
BEGIN
-- Get Add Job message for site in appropriate language / wording for site
SELECT @ChangeCode = 1 , @Action = dbo.udf_GET_MESSAGE(@SitePK, 455)
BEGIN TRAN
-- short example of a new job
INSERT INTO JOBS
(SiteFK, ClientFK,JobTitle,JobDescription)
VALUES
(@SitePK, @ClientPK, @JobTitle, @JobDescription)
-- get new jobID
SELECT @errors = @@error, @rowcount = @@rowcount, @JobPK = SCOPE_IDENTITY()
-- if error
IF @errors != 0 OR @rowcount<>1
BEGIN
-- get correct update error message
SELECT @message = dbo.udf_GET_ERROR_MESSAGE(@SitePK, 102)
-- go to error handler
GOTO HANDLE_ERROR
END
-- if application folders are on for the site create a new group
IF @ApplicationFolders = 1
BEGIN
-- ensure group is clean without a check as that just wastes time IF it does exist already, obviously if it doesn't then it won't do anthing but the fields are indexed
DELETE
FROMGROUPS
WHEREJobFK = @JobPK
AND GroupType = 'DJA'
AND ClientFK = @ClientPK
-- doesn't matter if nothing was deleted
INSERT INTO GROUPS
(GroupName, ClientFK, GroupType, JobFK)
VALUES
(@JobTitle, @ClientPk, 'DJA', @JobPK)
-- ensure no errors and set message if there was a group error
SELECT @errors = @@error, @rowcount = @@rowcount
IF @errors != 0 OR @rowcount <> 1
BEGIN
-- get correct group inset error message
SELECT @message = dbo.udf_GET_ERROR_MESSAGE(@SitePK, 220)
-- go to error handler
GOTO HANDLE_ERROR
END
END
END
-- other work related to multiple linked sites
-- other code related to handle PENDING (non live) jobs
-- if we get here NO ERRORS so COMMIT the transaction
IF @@TRANCOUNT = 0
COMMIT TRAN
-- set return values including a success message and the return code for the application
SELECT @success = 1, @message = dbo.udf_GET_MESSAGE(@SitePK, 103)
-- skip over error handler and exit proc
GOTO EXIT_PROC
END
-- error handler code - cannot be in here if NO ERROR which is why GOTO jumps OVER it
HANDLE_ERROR:
-- if we have an open transaction close it
IF @@TRANCOUNT > 0
ROLLBACK TRAN
-- if we don't have an error message get a default on e.g "Job failed to save please try back later"
IF ISNULL(@message,'') = ''
SELECT @message = dbo.udf_GET_ERROR_MESSAGE(@SitePK, 100)
-- we failed so succes is 0
SELECT @success = 0
-- exit the stored proc
EXIT_PROC:
-- before we exit the proc we log any changes to the table and any error messages if thee were any
IF INSULL(@Details,'') != ''
BEGIN
EXEC [dbo].[usp_sql_add_job_change_record] @SitePK, @JobPK, @ChangeCode, @Action, @Details, @Success, @Message
END
-- return to caller
SELECT cast(@Success as int) as Success, isnull(@JobPK,0) as JobPK, @message as [message], @CheckMode as checkmode
-- use success as return code in case we are using an ADO stored proce that relies on return codes
RETURN @Success
Also if I had a stored proc that saved MULTIPLE jobs at the same time e.g @JobID was not ONE integer but a comma delimited list of integers to save in one go, how could I not use a cursor to loop through them all when for EACH JOB I have statements to check for duplicate submits, finding out what has changed in the job for reporting, plus logging that information with another stored proc call that is used across the site for logging info regarding changes to important records.
Cursors have their time and place especially if you have to do procedural type procedure like the one above. I have seen people just try and replace cursors with temporary tables with identity fields and then WHILE loops selecting the next ID record before doing the middle content of a proc like the one above before incrementing the loop counter. This is just creating a cursor in another manner and if you have to do a procedural type proc then a FAST_FORWARD LOCAL CURSOR is designed for looping from start to end so why try and replace it.
If anyone can show me how I can replace all the contents of the proc above without GOTO statements and why the format used is bad I would like to know.
Also if you can imagine having multiple jobs to handle in the proc rather than one, so @JobIDs varchar(1000) (e.g 12,13,14), @JobTitle nvarchar(max) 'Test 1','Test 2','Test 3' (Same for Job Desc) e.g position N of each comma delimited list relates to position N of the other values I would be interested to see.
Each job needs the details of the changes done to it logged, the related groups checked and inserted/updated, plus the changes all logged with the stored proc currently residing in the EXIT_PROC part of the proc. In a cursor loop this would be at the bottom of the loop iteration.
And I don't use cursors if I can do it in a set based by the way.
Just when the data logic is more complicated than a single SELECT/UPDATE/INSERT statement and requires calls to other functions / procedures.
Thanks
Rob
Hey Rob, you could pretty easily rewrite this code using a TRY/CATCH block instead. the CATCH would automatically handle any errors that occur within the TRY block (as long as the severity is high enough). Therefore eliminating a lot of redundant code where you continuously check for @@ERROR.
I may be wrong here also, but in your proc above, specific errors that occur at the statement level (such as divide-by-zero) might halt immediately and never enter into the HANDLE_ERROR block. A Try/Catch would not have this issue. (I may be wrong here because it sometimes depends on instance-level settings, such as the default values that connections are given for XACT_ABORT/etc...).
I personally have not found a need for GOTO, where TRY/CATCH could not satisfy the same needs. Interested to hear if others feel differently?
June 3, 2015 at 6:56 am
Rob Reid-246754 (6/3/2015)
Regarding the GOTO statement, are people saying it has no place in TSQL either?
Yes that is my sentiment exactly. The GOTO has no relevance in t-sql since 2005 when the try/catch was introduced. Just like other programming languages when the try/catch is introduced the GOTO needs to stop being used.
_______________________________________________________________
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/
June 3, 2015 at 7:56 am
IMO, cursors are there to be used for admin scripts to be executed manually. They can be a time saver.
The philosophy I attempt to maintain in my SQL Server environment is, if a T-SQL script (or a sproc) is used either by an application or is being automated (scheduler, sql job) and for SOME reason (good or bad) it must use a cursor, then I make my case to pull that code out of the SQL Server environment and have another language (C#, Java, Powershell, etc) do that work. The way I see it, loopie code should not be written with SQL in a relational database. Database is designed to store and retrieve data fast. So use SQL for that. Want to do fast loopie stuff, do it outside of SQL Server. Much faster and less load on the db. Yes, more work. But, if the loopie code is becoming a repeatable process, then it should be outside of the database. Just pull the data out that you need, do the work and stuff back the changes. If that is not good enough, then a service broker/messaging solution should be considered. I suppose, all I'm trying to say is use the proper technology for the task. SQL was not designed to do procedural programming, so don't use it for that. Yes, IF and WHILE statements and cursors are there, but they are extensions to SQL provided for administrative purposes. This is also a good reason to keep business logic outside of the database. When IF and WHILE statements are in a sproc, business logic begins to creep in. Those are the things I keep a close eye on to make sure that doesn't happen.
Note: I'm using loopie code to mean RBAR.
June 3, 2015 at 9:20 am
Ok, I totally get that as I do use TRY CATCH and all the ERROR_MESSAGE() ERROR_LINE() ERROR_NUMBER() ERROR_PROCEDURE() functions to get detailed errors in TRY CATCH statements and log the errors into a table before a ROLLBACK but then that doesn't still stop the need for a "get out" statement as the errors could be NON SQL Errors e.g DATA/BUSINESS LOGIC such as the company who is placing the job has a setting to only allow one job per job reference or unique job titles etc.
These are not TSQL errors but would need TSQL to check the logic and then still I can see a need if there is an error like that to jump to a HANDLE_ERROR statement and an EXIT_PROC statement to get out the proc.
A TRY/CATCH statement would be no use in this case as the errors are not SQL ones.
So what would you do there?
Just RETURN from the proc at the point where you find the logic error even if it means duplicate code?
OR have a single HANDLE_ERROR section where you handle the return settings and return error messag
-- some logic that raises a NON SQL error
-- check for dupe JobRef
IF EXISTS(SELECT JobREF FROM JOBS WHERE SiteFK = @SitePK AND ClientFK = @ClientPK)
BEGIN
-- dupe job ref
SELECT @ErrorMessage = 'There is already a job with this job ref in the system'
GOTO HANDLE_ERROR
END
-- do error handling in TRY CATCH
BEGIN TRY
BEGIN TRAN
-- code to save job
-- other code
END TRY
BEGIN CATCH
-- log error before rollback
EXEC usp_sql_log_error ERROR_MESSAGE(), ERROR_PROCEDURE(), ERROR_NUMBER(), ERROR_LINE(), @SitePK
IF @@TRANCOUNT > 0
ROLLBACK TRAN
END CATCH
IF @@TRANCOUNT > 0
COMMIT TRAN
-- everything good so skip over error handle section
GOTO EXIT PROC:
HANDLE_ERROR:
SELECT @Success = 0, @ErrorMessage = ISNULL(@DefaultErrorMessage)
EXIT_PROC:
SELECT @Success = 1, @Message = 'Job Saved'
What would you do in this scenario?
June 3, 2015 at 9:34 am
Rob Reid-246754 (6/3/2015)
Ok, I totally get that as I do use TRY CATCH and all the ERROR_MESSAGE() ERROR_LINE() ERROR_NUMBER() ERROR_PROCEDURE() functions to get detailed errors in TRY CATCH statements and log the errors into a table before a ROLLBACK but then that doesn't still stop the need for a "get out" statement as the errors could be NON SQL Errors e.g DATA/BUSINESS LOGIC such as the company who is placing the job has a setting to only allow one job per job reference or unique job titles etc.These are not TSQL errors but would need TSQL to check the logic and then still I can see a need if there is an error like that to jump to a HANDLE_ERROR statement and an EXIT_PROC statement to get out the proc.
A TRY/CATCH statement would be no use in this case as the errors are not SQL ones.
So what would you do there?
Just RETURN from the proc at the point where you find the logic error even if it means duplicate code?
OR have a single HANDLE_ERROR section where you handle the return settings and return error messag
-- some logic that raises a NON SQL error
-- check for dupe JobRef
IF EXISTS(SELECT JobREF FROM JOBS WHERE SiteFK = @SitePK AND ClientFK = @ClientPK)
BEGIN
-- dupe job ref
SELECT @ErrorMessage = 'There is already a job with this job ref in the system'
GOTO HANDLE_ERROR
END
-- do error handling in TRY CATCH
BEGIN TRY
BEGIN TRAN
-- code to save job
-- other code
END TRY
BEGIN CATCH
-- log error before rollback
EXEC usp_sql_log_error ERROR_MESSAGE(), ERROR_PROCEDURE(), ERROR_NUMBER(), ERROR_LINE(), @SitePK
IF @@TRANCOUNT > 0
ROLLBACK TRAN
END CATCH
IF @@TRANCOUNT > 0
COMMIT TRAN
-- everything good so skip over error handle section
GOTO EXIT PROC:
HANDLE_ERROR:
SELECT @Success = 0, @ErrorMessage = ISNULL(@DefaultErrorMessage)
EXIT_PROC:
SELECT @Success = 1, @Message = 'Job Saved'
What would you do in this scenario?
Something like this....
BEGIN TRY
DECLARE @Sev INT = 16; -- change this value to 7 to raise exception but not drop into the CATCH block.
DECLARE @Tst BIT = 1; -- change this value to 0 for unexpected exception
IF @Tst = 'TRUE'
RAISERROR('Raise an error', @Sev, 1); -- NOTE: Severity 11-19 will cause execution to jump to CATCH block
ELSE
SELECT 1/0; -- force unexpected exception
SELECT 'This statement will only execute when severity is not 11-19. CATCH block bypassed.';
END TRY
BEGIN CATCH
IF ERROR_NUMBER() >= 50000 -- Check for user defined messages (sp_addmessage).
PRINT 'Expected exception';
ELSE
PRINT 'Unexpected exception';
-- THROW; -- if needed
END CATCH
June 3, 2015 at 9:48 am
To add to what qbrt is (I think rightly) pointing out, you can check ERROR_NUMBER() in your CATCH block and if it is 50000, you'll know that you raised it. If you want to differentiate how you handle "expected" errors that you raised and "unexpected" T-SQL errors.
June 3, 2015 at 10:01 am
Brian J. Parker (6/3/2015)
To add to what qbrt is (I think rightly) pointing out, you can check ERROR_NUMBER() in your CATCH block and if it is 50000, you'll know that you raised it. If you want to differentiate how you handle "expected" errors that you raised and "unexpected" T-SQL errors.
Correct. Actually, any number above 50,000 if defining user messages with sp_addmessage sproc.
Thanks for the clarification, Brian. I also updated the code snippet to reflect the expected/unexpected message check.
Viewing 15 posts - 91 through 105 (of 215 total)
You must be logged in to reply to this topic. Login to reply