How Do I Fix This Proc?

29 Sep 2005 10:29 PM

Hal Heinrich

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.

29 Sep 2005 11:30 PM

David Browne

"Hal Heinrich" <HalHeinr***@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

30 Sep 2005 4:36 PM

Hal Heinrich

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 week and will post the results here.

Thanks again,
Hal Heinrich
VP Technology
Aralan Solutions Inc.

Show quote

"David Browne" wrote:

>
> "Hal Heinrich" <HalHeinr***@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
>
>
>
>
>

2 Oct 2005 6:37 PM

Fred

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

Show quote

"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 week and will post the results here.
>
> Thanks again,
> Hal Heinrich
> VP Technology
> Aralan Solutions Inc.
>
>
> "David Browne" wrote:
>
> >
> > "Hal Heinrich" <HalHeinr***@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
> >
> >
> >
> >
> >

2 Oct 2005 7:21 PM

David Browne

"Fred" <F***@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