Risk in the stored procedure below

Question

Post reply

Risk in the stored procedure below

leos1981

SSC Journeyman

Points: 93
More actions
January 5, 2006 at 9:37 pm

#90428

Hai, today i found a store procedure created by other programmer in the current project I worked with. The SP is used to generate a ID for all tables.
The contents in the SP is like below:
CREATE PROCEDURE Get_New_ID
@Prefix varchar(2)
AS
select @currentId = currId from TABLE_ID
update TABLE_ID set currId = @currentId + 1
select @Prefix + convert(varchar(10),(@currentId + 1)) as New Id
GO
Can I know will this SP have a risk when that is a lot of users using this SP at the same time? Will the duplicate ID problem happened?

Viewing 7 posts - 1 through 6 (of 6 total)

You must be logged in to reply to this topic. Login to reply

Charles Otten Default port Points: 1475 More actions · Answer 1

Two risks that I see:

One is no transaction so if a timeout or other issue happens you could end up with duplicates.

Two, yes to my knowledge you could end up with duplicates in this scenario. I could be wrong, but i think you have a risk here.

I would suggest creating an identity field instead, which doesn't have this concern.

If the phone doesn't ring...It's me.

David Webb-CDS SSCoach Points: 17398 More actions · Answer 2

If you're going to do it this way, I'd suggest the following structure to minimize locking:

CREATE PROCEDURE Get_New_ID

@Prefix varchar(2)

AS

begin tran

-- do the update to increment the counter and get a lock

update TABLE_ID set currId = currId + 1

-- put error checking in here

-- grab the updated value and commit the transaction

select @currentId = currId from TABLE_ID

commit

return

go

And then again, I might be wrong ...
David Webb

leos1981 SSC Journeyman Points: 93 More actions · Answer 3

Thank you for your help. I think I still need to use back the current method to generate the Id, coz It is a common function used by the system and the system already in the production for few years. I affraid changing the function may cause unexpected bug occurred. Anywhere, I will take this as an experiance.

Thank you

Mark Hickin Say Hey Kid Points: 693 More actions · Answer 4

Here is a better approach.

CREATE PROCEDURE Get_New_ID

@Prefix varchar(2)

AS

DECLARE @New_id INT --Assuming this is an int

UPDATE TABLE_ID

SET @new_id = currId = @currentId + 1

select @new_id

GO

Uses a little known construct, that does the update and assigns to a variable in one go. Try it, it works, and its covered by an implicit transaction. Still a great way to cause blocking and deadlocks in your database though. I never really understood why people need database wide keys. IMHO keys should be unique within the scope of a table in 99% of cases.

Mark

Mark Hickin Say Hey Kid Points: 693 More actions · Answer 5

Sorry, I had a typo. Here is an example:

CREATE

TABLE ids ( id int)

GO

CREATE

PROCEDURE get_id

AS

DECLARE

@newid int

UPDATE

ids

SET

@newid = id = id+1

SELECT

@newid

GO

INSERT

INTO ids VALUES (0)

GO

EXEC

Get_id

EXEC

Get_id

EXEC

Get_id

EXEC

Get_id

Ian Yates SSCoach Points: 19738 More actions · Answer 6

You could also create a table called KEYS which has a single identity field. Then your stored proc just does an insert to this table and retrieves the SCOPE_IDENTITY value... This should ensure that all users get unique keys all of the time. You can even periodically clear out the table