September 16, 2009 at 8:18 am
Hello everyone,
like I have said before, I am a Client-side developer and I am just beggining to port some ADO code to T-SQL.
This is a sp that is supposed to read a table (20 rows...) that holds number ranges and return the next number in sequence
I try to use a timestamp much in the way that we use it in client programming to control concurrency.
just take a look and make any comments based on your experience because I have none !
CREATE PROCEDURE [dbo].[usp_GetNextNumber]
(@pNumberRange AS NVARCHAR(20), @pNextValue AS INT OUTPUT)
AS
BEGIN
DECLARE @IStartedTrans AS BIT
DECLARE @rc AS TINYINT
DECLARE @NRTimestamp AS TIMESTAMP
DECLARE @NextNumber AS INT
DECLARE @Msg AS NVARCHAR(500)
DECLARE @MaxValue AS INT
BEGIN TRY
SET @rc = 0
SET @IStartedTrans = 0
IF @@TRANCOUNT = 0
BEGIN
BEGIN TRANSACTION;
SET @IStartedTrans = 1
END
ELSE
SET @IStartedTrans = 0
SELECT @NRTimestamp = binTimestamp,
@pNextValue = nCurrValue,
@MaxValue = nMaxNum
FROM dbo.stblNumberRanges
WHERE cNROB = @pNumberRange
IF @@ROWCOUNT <> 1
BEGIN
SET @rc = 0
SET @NextNumber = 0
SET @Msg = 'Number range ' + @pNumberRange + ' was not found'
RAISERROR (@Msg, 16, 1)
END
SET @pNextValue = @pNextValue + 1
IF @pNextValue > @MaxValue
BEGIN
SET @rc = 0
SET @NextNumber = 0
SET @Msg = 'Number Range ' + @pNumberRange +
' exceeded maximum value'
RAISERROR (@Msg, 16, 1)
END
UPDATE dbo.stblNumberRanges
SET nCurrValue = @pNextValue
WHERE cNROB = @pNumberRange
AND binTimestamp = @NRTimestamp
IF @@ERROR <> 0
OR @@ROWCOUNT <> 1
BEGIN
SET @rc = 0
SET @NextNumber = 0
SET @Msg = 'Number Range ' + @pNumberRange +
' was updated by another user'
RAISERROR (@Msg, 16, 1)
END
IF @IStartedTrans = 1
BEGIN
SET @IStartedTrans = 0
COMMIT TRANSACTION;
END
SET @rc = 1
RETURN @rc
END TRY
BEGIN CATCH
IF @IStartedTrans = 1
BEGIN
SET @IStartedTrans = 0
ROLLBACK TRANSACTION;
END
SET @rc = 0
IF LEN(@Msg) > 0
RAISERROR (@Msg, 16, 1)
ELSE
BEGIN
SET @Msg = N'Error: ' + RTRIM(LTRIM(CAST(ERROR_NUMBER() AS NVARCHAR(500))))
+ ' ' + ERROR_MESSAGE()
RAISERROR (@Msg, 16, 1)
END
END CATCH
END
September 16, 2009 at 10:40 am
You do not need most of that code. You can do it in one statement, and one statement is an atomic transaction.
CREATE PROCEDURE dbo.usp_GetNextNumber @pNumberRange AS nvarchar(20), @pNextValue AS int OUTPUT
AS
SET NOCOUNT ON;
DECLARE @msg nvarchar(50);
BEGIN TRY
UPDATE dbo.stblNumberRanges SET @pNextValue = nCurrValue = nCurrValue + 1
WHERE cNROB = @pNumberRange;
END TRY
BEGIN CATCH
SET @pNextValue = 0;
SET @Msg = N'Number Range ' + @pNumberRange + N' exceeded maximum value';
RAISERROR (@Msg, 16, 1);
END CATCH;
IF @pNextValue IS NULL BEGIN
SET @pNextValue = 0;
SET @Msg = N'Number Range ' + @pNumberRange + N' was not found';
RAISERROR (@Msg, 16, 1);
END
September 16, 2009 at 10:55 am
...I am speechless 🙂
thanks for your input, but how does this handle concurrency? I need to have sequential numbering are you sure this will leave no blanks in the numbering?
what will happen if two different users try to get a number from the same number range, and SQL server will try to update the same record?
sorry for the question but when using ADO, then it handles the updates using timestamp values.
other than that, , not only does your suggestion look a lot more elegant but it should be at least twice as fast as well.
thank again for your feedback
September 16, 2009 at 11:34 am
SQL Server takes care of concurrency issues for you, which is reason to try and use it instead of write your own. As I write, one statement is an atomic transaction. You probably do not need more than the two columns in the dbo.stblNumberRanges table, and cNROB should be the clustered primary key.
Myself I would not use the TRY..CATCH, instead just
ALTER PROCEDURE dbo.usp_GetNextNumber @pNumberRange AS nvarchar(20), @pNextValue AS int OUTPUT
AS
SET NOCOUNT ON;
UPDATE dbo.stblNumberRanges SET @pNextValue = nCurrValue = nCurrValue + 1
WHERE cNROB = @pNumberRange;
because errors with messages already will be returned to calling program. If output parameter is returned null but there is no error, you know that @pNumberRange was not found.
I cheated with coding any error as overflow. Better example is:
ALTER PROCEDURE dbo.usp_GetNextNumber @pNumberRange AS nvarchar(20), @pNextValue AS int OUTPUT
AS
SET NOCOUNT ON;
DECLARE @msg nvarchar(500);
BEGIN TRY
UPDATE dbo.stblNumberRanges SET @pNextValue = nCurrValue = nCurrValue + 1
WHERE cNROB = @pNumberRange;
END TRY
BEGIN CATCH
SET @pNextValue = 0;
IF ERROR_NUMBER() = 8115
SET @Msg = N'Number Range ' + @pNumberRange + N' exceeded maximum value';
ELSE
SET @Msg = N'Unexpected SQL error - ' + ERROR_MESSAGE()
RAISERROR (@Msg, 16, 1);
END CATCH;
IF @pNextValue IS NULL BEGIN
SET @pNextValue = 0;
SET @Msg = N'Number Range ' + @pNumberRange + N' was not found';
RAISERROR (@Msg, 16, 1);
END
September 16, 2009 at 11:38 am
d viz (9/16/2009)
...I am speechless 🙂thanks for your input, but how does this handle concurrency? I need to have sequential numbering are you sure this will leave no blanks in the numbering?
what will happen if two different users try to get a number from the same number range, and SQL server will try to update the same record?
sorry for the question but when using ADO, then it handles the updates using timestamp values.
other than that, , not only does your suggestion look a lot more elegant but it should be at least twice as fast as well.
thank again for your feedback
It'll handle some extreme concurrency just fine and it won't leave any gaps in the numbering unless, like an IDENTITY column, a rollback of a new row is accomplished. Depending on how they are used and whether code makes a mistake or does an inappropriate rollback, sequence tables of this nature can get out of sync and that makes for some serious problems. Although using the 3 part "Quirky Update" method that Hans' good code has in it certainly helps prevent deadlocks, the code that calls the procedure should never include the procedure call in an explicit transaction.
The major problem with sequence tables of this nature is that no one considers what happens if you need to "reserve" more than one ID. When I get home, I'll see if I can find the code in my archives that does consider a batch of multiple inserts.
Unless you have some big time probles between IDENTITY and REPLICATION, the preferred method would be to get away from an Oracle-like sequence table of this nature and use an auto-numbering feature like an IDENTITY column.
--Jeff Moden
Change is inevitable... Change for the better is not.
September 16, 2009 at 11:52 am
Jeff Moden (9/16/2009)
The major problem with sequence tables of this nature is that no one considers what happens if you need to "reserve" more than one ID. When I get home, I'll see if I can find the code in my archives that does consider a batch of multiple inserts.Unless you have some big time probles between IDENTITY and REPLICATION, the preferred method would be to get away from an Oracle-like sequence table of this nature and use an auto-numbering feature like an IDENTITY column.
🙂
I considered writing that also. I do so by having a third parameter in the proc that is the number of rows being inserted, and it defaults to 1.
September 16, 2009 at 12:27 pm
Although using the 3 part "Quirky Update" method that Hans' good code has in it certainly helps prevent deadlocks, the code that calls the procedure should never include the procedure call in an explicit transaction
ok just a few comments:
1. first of all I don't understand what quirky "means" 🙂 (english is a second language for me)
2. No requirement whatsoever for more than 1 value reservation - so I guess that simplifies things
3. This procedure will ALWAYS run in an explicit transaction - the client program request a new value from the number range (one update) and then uses this value as a surrogate key to insert or update one or more tables (invoice ID is a classic example I believe). The transactions are always kept short (I know because I have reviewed the whole source code).
Now holes are maybe an issue - but correct me if I am wrong,
if the row is locked until each transaction commits, then no holes will ever be created because the number range cannot be updated by another transaction until the first one commits or rollsback.
The users aren't hitting the table all that often and even although the chance of deadlock exists, I believe that row level locking should prevent them most of the time.
so when you say that the procedure call should never be called from within an explicit transaction I understand that you are saying that this can lead to deadlocks - right?
September 16, 2009 at 2:38 pm
"Quirky" update is a name assigned to 3 part updates by Phil Factor. In this case, "Quirky" means that it operates in a different manner than most people expect.
So far as always having the sequence table code as part of an explicit transaction, I'd say that you need to prepare to monitor for deadlocks. The "Quirky" update will help avoid those but we had similar code that cause 640 deadlocks a day.
Also, you say there is no requirement for more than 1 ID at a time and while that may be true now, prepare yourself because that requirement will change in the not so distant future. It just happens.
--Jeff Moden
Change is inevitable... Change for the better is not.
September 21, 2009 at 4:41 am
A follow up for the record gentlemen:
as of a couple of days the stored procedure runs in production with minor modifications and no problems whatsoever.
Thank you again for your help!
September 21, 2009 at 6:35 am
Very cool... thanks for the feedback.
--Jeff Moden
Change is inevitable... Change for the better is not.
Viewing 10 posts - 1 through 9 (of 9 total)
You must be logged in to reply to this topic. Login to reply