How Do I Fix This Proc?

In a multi-user environment, this proc is expected to return distinct values
for concurrent users - but it doesn't, some duplicates are returned. I
thought that the 'BEGIN TRAN/COMMIT' would provide the required locking - I'
m
guessing that's where I went wrong. How do I fix this proc? The DDL for the
tables involved is included.
PROCEDURE procNextKey_Well
( @.NK int OUTPUT
) AS
BEGIN
DECLARE @.NK2 int
SELECT @.NK = COALESCE(MAX(wellId), 0) FROM dbo.well
SELECT @.NK2 = maxId FROM dbo.tbTableMaxId
WHERE (tableName = N'well')
BEGIN TRAN
IF @.NK2 IS NULL
BEGIN
SET @.NK = @.NK + 1
INSERT INTO dbo.tbTableMaxId
(tableName, maxId)
ELECT N'well', @.NK
END
ELSE
BEGIN
IF @.NK2 > @.NK
SET @.NK = @.NK2 + 1
ELSE
SET @.NK = @.NK + 1
UPDATE dbo.tbTableMaxId
SET maxId = @.NK
WHERE (tableName = N'well')
END
COMMIT
END
CREATE TABLE dbo.tbTableMaxId
( tableName nvarchar(50) NOT NULL,
maxId int NOT NULL,
PRIMARY KEY (tableName)
)
CREATE TABLE dbo.well
( wellId int NOT NULL,
wellName nvarchar(50) NULL,
PRIMARY KEY (wellId)
)
Thanks in advance for your help,
Hal Heinrich
VP Technology
Aralan Solutions Inc."Hal Heinrich" <HalHeinrich@.discussions.microsoft.com> wrote in message
news:3AF75018-7A96-403F-877E-8886D434975C@.microsoft.com...
> In a multi-user environment, this proc is expected to return distinct
> values
> for concurrent users - but it doesn't, some duplicates are returned. I
> thought that the 'BEGIN TRAN/COMMIT' would provide the required locking -
> I'm
> guessing that's where I went wrong. How do I fix this proc? The DDL for
> the
> tables involved is included.
>
Basically strict serialization of the whole procedure will be required to
make this correct, so I hope you don't have to support concurrent inserts.
Which is the main reason why IDENTITY columns exist and you shouldn't try to
emulate them with custom code.
David
Here is your fix:
PROCEDURE procNextKey_Well
( @.NK int OUTPUT
) AS
BEGIN
BEGIN TRAN
DECLARE @.NK2 int
SELECT @.NK = COALESCE(MAX(wellId), 0) FROM dbo.well (tablockx,holdlock)
SELECT @.NK2 = maxId FROM dbo.tbTableMaxId (tablockx,holdlock)
WHERE (tableName = N'well')
IF @.NK2 IS NULL
BEGIN
SET @.NK = @.NK + 1
INSERT INTO dbo.tbTableMaxId
(tableName, maxId)
ELECT N'well', @.NK
END
ELSE
BEGIN
IF @.NK2 > @.NK
SET @.NK = @.NK2 + 1
ELSE
SET @.NK = @.NK + 1
UPDATE dbo.tbTableMaxId
SET maxId = @.NK
WHERE (tableName = N'well')
END
COMMIT
END|||David,
Thank you for your response. I've modified your solution to avoid locking
the well table as follows:
PROCEDURE procNextKey_Well
( @.NK int OUTPUT
) AS
BEGIN
SET NOCOUNT ON
DECLARE @.NK2 int
SELECT @.NK = COALESCE(MAX(wellId), 0) FROM dbo.well
BEGIN TRAN
SELECT @.NK2 = maxId FROM dbo.tbTableMaxId (tablockx, holdlock)
WHERE (tableName = N'well')
IF @.NK2 IS NULL
BEGIN
SET @.NK = @.NK + 1
INSERT INTO dbo.tbTableMaxId
(tableName, maxId)
SELECT N'well', @.NK
END
ELSE
BEGIN
IF @.NK2 > @.NK
SET @.NK = @.NK2 + 1
ELSE
SET @.NK = @.NK + 1
UPDATE dbo.tbTableMaxId
SET maxId = @.NK
WHERE (tableName = N'well')
END
COMMIT
END
I'll be testing this next w and will post the results here.
Thanks again,
Hal Heinrich
VP Technology
Aralan Solutions Inc.
"David Browne" wrote:

> "Hal Heinrich" <HalHeinrich@.discussions.microsoft.com> wrote in message
> news:3AF75018-7A96-403F-877E-8886D434975C@.microsoft.com...
> Basically strict serialization of the whole procedure will be required to
> make this correct, so I hope you don't have to support concurrent inserts.
> Which is the main reason why IDENTITY columns exist and you shouldn't try
to
> emulate them with custom code.
> David
> Here is your fix:
> PROCEDURE procNextKey_Well
> ( @.NK int OUTPUT
> ) AS
> BEGIN
> BEGIN TRAN
> DECLARE @.NK2 int
> SELECT @.NK = COALESCE(MAX(wellId), 0) FROM dbo.well (tablockx,holdlock)
> SELECT @.NK2 = maxId FROM dbo.tbTableMaxId (tablockx,holdlock)
> WHERE (tableName = N'well')
>
> IF @.NK2 IS NULL
> BEGIN
> SET @.NK = @.NK + 1
> INSERT INTO dbo.tbTableMaxId
> (tableName, maxId)
> ELECT N'well', @.NK
> END
> ELSE
> BEGIN
> IF @.NK2 > @.NK
> SET @.NK = @.NK2 + 1
> ELSE
> SET @.NK = @.NK + 1
> UPDATE dbo.tbTableMaxId
> SET maxId = @.NK
> WHERE (tableName = N'well')
> END
> COMMIT
> END
>
>
>|||Hello Heinrich!
I think, it would a better idea if you try to lock only ,row for N'well'
table,
I thin you should use an rowlock hint
"Hal Heinrich" wrote:
> David,
> Thank you for your response. I've modified your solution to avoid locking
> the well table as follows:
> PROCEDURE procNextKey_Well
> ( @.NK int OUTPUT
> ) AS
> BEGIN
> SET NOCOUNT ON
> DECLARE @.NK2 int
> SELECT @.NK = COALESCE(MAX(wellId), 0) FROM dbo.well
> BEGIN TRAN
> SELECT @.NK2 = maxId FROM dbo.tbTableMaxId (tablockx, holdlock)
> WHERE (tableName = N'well')
> IF @.NK2 IS NULL
> BEGIN
> SET @.NK = @.NK + 1
> INSERT INTO dbo.tbTableMaxId
> (tableName, maxId)
> SELECT N'well', @.NK
> END
> ELSE
> BEGIN
> IF @.NK2 > @.NK
> SET @.NK = @.NK2 + 1
> ELSE
> SET @.NK = @.NK + 1
> UPDATE dbo.tbTableMaxId
> SET maxId = @.NK
> WHERE (tableName = N'well')
> END
> COMMIT
> END
> I'll be testing this next w and will post the results here.
> Thanks again,
> Hal Heinrich
> VP Technology
> Aralan Solutions Inc.
>
> "David Browne" wrote:
>|||"Fred" <Fred@.discussions.microsoft.com> wrote in message
news:8A55A9E5-C188-4DCF-BDD9-656AD810C31B@.microsoft.com...
> Hello Heinrich!
> I think, it would a better idea if you try to lock only ,row for N'well'
> table,
> I thin you should use an rowlock hint
>
Why? And what row do you propose locking?
David