July 14, 2010 at 2:51 pm
Hi All.
I have a table:
Table GPK
gpk Char(30) -- Name of tables
gpk_ID INT -- Last PK
CREATE TABLE gPK (gpk CHAR(30), gpk_ID INT)
INSERT gpk (gpk, gpk_id) VALUES ('HOLDER', 1)
I created the following stored procedure:
IF OBJECT_ID ( 'dbo.usp_GenPK', 'P' ) IS NOT NULL
DROP PROCEDURE dbo.usp_GenPK;
GO
CREATE PROCEDURE dbo.usp_GenPK( @table_name char(128) = '')
AS
BEGIN
DECLARE @oTbl TABLE (pk ID)
DECLARE @return_ID ID
begin transaction GPK
update GPK set gpk_ID = gpk_ID + 1
output INSERTED.gpk_ID INTO @oTbl
WHERE gpk_pk = 'HOLDER'
SELECT @Return_ID = pk FROM @oTbl
commit transaction GPK
RETURN @Return_ID
END
Problem is that I can seem to get the return value into a variable. I tried this as a function but it wouldn't run it because of the transaction and update statements.
Any ideas?
Thanks,
Mike
July 14, 2010 at 3:07 pm
What exactly are you trying to do?
I don't see the input parameter @table_name being used anywhere.
The condition WHERE gpk_pk = 'HOLDER' will fail since gpk_pk is an INT data type.
The script will fail as soon as you have more than one row in GPK with a value gpk='HOLDER' since you'd try to assign more than one value to the variable @Return_ID.
Using GPK as the name of a table, a column of the table and as a transaction name is hopefully just for the sample code.... Otherwise I'd consider it really bad practice...
If you'd tell us what you're looking for we might be able to provide an alternative.
July 14, 2010 at 3:13 pm
I have legacy code that does not allow for identity columns, thus I have a table that holds the last "PK" value. I am getting occasional update issues. I need to create a stored proc that locks the "record" for a table, adds 1 to the last used #, unlocks the record and then returns the new ID.
I know that SQL doesn't really do record level locking, but I think a transaction would work.
Does this help?
Mike
July 14, 2010 at 3:25 pm
mike 57299 (7/14/2010)
I have legacy code that does not allow for identity columns, thus I have a table that holds the last "PK" value. I am getting occasional update issues. I need to create a stored proc that locks the "record" for a table, adds 1 to the last used #, unlocks the record and then returns the new ID.I know that SQL doesn't really do record level locking, but I think a transaction would work.
Does this help?
Mike
Not really...
If it's legacy code, get the vendor in to fix the issue.
What would you need the stored proc for in the first place? Are you planning to insert rows without using the vendor's app? If so, I strongly vote against it. You might break the code or data integrity.
July 14, 2010 at 3:40 pm
Actually, I am the vendor and I don't have the time right now to rewrite all the code to handle identity columns. Currently, the code has to 'generate' the pk for each record.
July 14, 2010 at 3:52 pm
mike 57299 (7/14/2010)
Actually, I am the vendor and I don't have the time right now to rewrite all the code to handle identity columns. Currently, the code has to 'generate' the pk for each record.
OUCH!!!! :sick:
I strongly advice you to take the time to get your code and table structure right! And I don't say it just because I'm usually a customer being faced with stuff like you're trying to implement occasionally.
There is no valid reason whatsoever to implement this kind of code. Especially not with a reason like "I don't have the time right now".
July 14, 2010 at 4:11 pm
I agree with Lutz.
__________________________________________________
Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills
July 14, 2010 at 4:31 pm
Do you mean that you are inserting the PK value into each table and can't rewrite the inserts?
Honestly how long can it take, and if this is an update issue, perhaps you might want to take the time and fix it. Otherwise you're burying things deeper and potentially going to have an issue later as clients grow.
The other alternative is have you thought about a single column int, identity table that shadows each regular table? Drop an insert there, grab the identity, use it in your regular table. Then you would have a solid solution.
July 14, 2010 at 4:33 pm
In order to get the value, you need to grab the output from the stored proc, like this: DECLARE @newID int
EXEC @NewID = dbo.usp_GenPK
SELECT @newID
I used INTs since I don't have the ID data type...
Grabbing a RETURN value this way is usually reserved for error codes, usually to tell the calling app/proc whether or not the procedure completed successfully. A better way to pass the data back is to use an OUTPUT parameter, like this:IF OBJECT_ID ( 'dbo.usp_GenPK', 'P' ) IS NOT NULL
DROP PROCEDURE dbo.usp_GenPK;
GO
CREATE PROCEDURE dbo.usp_GenPK( @table_name char(128) = '', @NewID int OUTPUT)
AS
BEGIN
DECLARE @oTbl TABLE (pk int)
DECLARE @return_ID int
begin transaction GPK
update GPK set gpk_ID = gpk_ID + 1
output INSERTED.gpk_ID INTO @oTbl
WHERE gpk = 'HOLDER'
SELECT @NewID = pk FROM @oTbl
commit transaction GPK
END
DECLARE @newID int
EXEC dbo.usp_GenPK '',@NewID OUTPUT
SELECT @newID
Again, I swapped INT for ID.
I do agree with some of the other comments though, that making changes to your code would be very beneficial. I've seen and used a system similar to this before and it was quite the performance bottleneck. I'm also aware of the struggle to make large changes to an existing system, so I have sympathy. It will be worth the performance boost though. Good luck!
Chad
July 14, 2010 at 5:07 pm
Since the posted table GPK has no unique constraint for gpk column you might end up with duplicates causing your code to fail.
Either a unique constraint is required for the GPK column or a TOP 1 clause when assigning the value to @NewID.
Regarding sympathy: mine is close to Zero. Honestly. The struggle to change the code is a one-time effort. To deal with the consequences (e.g. performance) is an all-time pain.
I'd expect to have an INSERT sproc for each table that might get data inserted from more than one code section (maybe even for every DML statement - "it depends"). If not, it's time to change that, too.
The OP deliberately puts the success of the product at risk just because of time constraints. And by success I'm talking about stuff like maintainability, performance, modification cost a.s.o.
July 14, 2010 at 5:42 pm
Chad,
Thank you for your help. I will adapt the code to work for me.
Mike
July 14, 2010 at 10:05 pm
This seems like a perfectly reasonable attempt to implement a Sequence Table to me.
The code can be simplified:
-- Sequence table, holds next PK per table
CREATE TABLE dbo.GPK
(
table_name SYSNAME NOT NULL PRIMARY KEY,
next_value INTEGER NOT NULL
);
GO
-- Example row
INSERT dbo.GPK (table_name, next_value) VALUES (N'dbo.MyTable', 0);
GO
-- Procedure to allocate a new PK
CREATE PROCEDURE dbo.usp_GenPK(@table_name SYSNAME, @NewID INT OUTPUT)
AS
BEGIN
UPDATE dbo.GPK
SET @NewID = next_value = next_value + 1
WHERE table_name = @table_name;
END;
GO
-- Test
DECLARE @NewID INTEGER;
EXEC dbo.usp_GenPK N'dbo.MyTable', @NewID OUTPUT;
SELECT @NewID;
edit: forgot the schema prefix
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
July 14, 2010 at 10:46 pm
Paul White NZ (7/14/2010)
This seems like a perfectly reasonable attempt to implement a Sequence Table to me.The code can be simplified:
-- Sequence table, holds next PK per table
CREATE TABLE dbo.GPK
(
table_name SYSNAME NOT NULL PRIMARY KEY,
next_value INTEGER NOT NULL
);
GO
-- Example row
INSERT dbo.GPK (table_name, next_value) VALUES (N'dbo.MyTable', 0);
GO
-- Procedure to allocate a new PK
CREATE PROCEDURE dbo.usp_GenPK(@table_name SYSNAME, @NewID INT OUTPUT)
AS
BEGIN
UPDATE GPK
SET @NewID = next_value = next_value + 1
WHERE table_name = @table_name;
END;
GO
-- Test
DECLARE @NewID INTEGER;
EXEC dbo.usp_GenPK N'dbo.MyTable', @NewID OUTPUT;
SELECT @NewID;
Classic T-SQL therem Paul. That's how I resolved more than 640 deadlocks a day on a sequence table at a previous job.
--Jeff Moden
Change is inevitable... Change for the better is not.
July 14, 2010 at 11:02 pm
Jeff Moden (7/14/2010)
Classic T-SQL there Paul. That's how I resolved more than 640 deadlocks a day on a sequence table at a previous job.
Thanks, Jeff. It is easy to get Sequence Tables wrong, as your experience showed.
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
July 14, 2010 at 11:57 pm
Paul,
Do I need to do an explicit Begin Trans / Commit to make sure there is no duplicate sequence #'s?
Mike
Viewing 15 posts - 1 through 15 (of 22 total)
You must be logged in to reply to this topic. Login to reply