June 8, 2009 at 4:29 am
First, my reference sources:
http://www.sqlservercentral.com/articles/Miscellaneous/checkyoursqlserveridentity/1993/
From the documentation above it appears that SELECT SCOPE_IDENTITY() will return the identity column of the row I just inserted into the table on the same connection - actually the return value of the SP. Would it be more correct to encapsulate the instructions inside a transaction and so a) ensure that if something bad happens the insert can be rolled back and b) ensure that I actually get the correct scope_identity, or would transactions not affect scope_identity?
CREATE PROCEDURE sp_InsertUpdateUser
@isUpdate AS BIT = 0,
@userid AS INT = NULL,
@lotsOfColumns AS various
AS
BEGIN
SET NOCOUNT ON ;
IF (@isUpdate = 0)
BEGIN
INSERT INTO
[dbo].[tbl_Users]
(
[lotsOfColumns]
)
VALUES
(
@lotsOfColumns
);
SELECT 1; -- I want to return the new userID here and I don't want to use a giant where clause. It just seems like there is a better way...
END
ELSE
BEGIN
UPDATE
[dbo].[tbl_Users]
SET
[tbl_Users].[lotsOfColumns]= @lotsOfColumns
WHERE
[tbl_users].[userid] = @userid;
SELECT 1; -- Indicating success
END
END
GO
As an aside (apologies to the mods if this question needs to be split from this thread) what could be a better way to indicate the success of an SP (most SPs, not just this one) other than "select 1;"? It's still early in the project so I can make changes now before they're cemented in. For SPs that return something useful like userID or howMuchWoodWouldAWoodchuckChuck, I return those, but for seemingly "bland" SPs like sp_InvalidateSessionBySessionKey that either work or don't work, what do you suggest?
June 8, 2009 at 7:55 am
SCOPE_IDENTITY is not going to be affected negatively or positively by being called within a transaction. I would recommend the transaction though, just because it's a better way to manage your data.
Instead of SELECT 1, which returns a result set, I'd suggest using RETURN. That returns an output parameter that is traditionally used to mark a successful or non-successful completion of a procedure.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
June 8, 2009 at 8:09 am
You may find it easier to use an OUTPUT clause in the INSERT statement
.
.
INSERT INTO [dbo].[tbl_Users] ( [lotsOfColumns] )
OUTPUT inserted.userid
VALUES(@lotsOfColumns);
.
.
____________________________________________________
Deja View - The strange feeling that somewhere, sometime you've optimised this query before
How to get the best help on a forum
http://www.sqlservercentral.com/articles/Best+Practices/61537June 8, 2009 at 12:17 pm
Yeah, I should have mentioned that. For anything beyond a single row insert, that's the way to go.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
June 9, 2009 at 1:20 am
@mark: Thanks, the output clause seems to suit my needs perfectly.
@grant: How can I use RETURN values in my application that is calling this stored procedure? When I execute the procedure xx (in the code sample below) from SSMS I just get "(1 row(s) affected)". Also, in my application's code (snippet below) I don't get anything back when I use RETURN.
From what I read in BOL "[m]ost stored procedures follow the convention of using the return code to indicate the success or failure of the stored procedure. The stored procedures return a value of 0 when no errors were encountered. Any nonzero value indicates an error occurred," so I am assuming that RETURN values are pretty common for SPs. To me, for what I am trying to do, using SELECT appears to best suit my purposes as it returns a result set - unless I am failing to see how to use a RETURN value in my application.
Additionally, how widespread is "[a]ny nonzero value indicates an error occurred?" Trying to think responsibly, how many DBA's will I be able to reduce to a tears by carrying on in my current ways of returning 1 for success?
CREATE TABLE [dbo].[bob](
[id] [int] IDENTITY(1,1) NOT NULL,
[name] [varchar](10) NOT NULL
)
INSERT INTO
[bob] ([name])
OUTPUT
INSERTED.id AS "userID"
VALUES
('bob')
INSERT INTO
[bob] ([name])
OUTPUT
INSERTED.id AS "userID"
VALUES
('fred')
INSERT INTO
[bob] ([name])
OUTPUT
INSERTED.id AS "userID"
VALUES
('etc')
CREATE PROCEDURE xx
AS
UPDATE bob SET NAME = 'blahblah' WHERE ID = 2; RETURN 0;
GO
exec xx
DataSet ds = new DataSet();
SqlCommand sqlComm = sqlConn.CreateCommand();
sqlComm.CommandType = CommandType.StoredProcedure;
sqlComm.CommandText = spName;
if (null != sqlParams)
{
foreach (SqlParameter sqlP in sqlParams)
{
sqlComm.Parameters.Add(sqlP);
}
}
SqlDataAdapter sda = new SqlDataAdapter(sqlComm);
sda.Fill(ds);
return ds;
June 9, 2009 at 5:34 am
If you use the result set to show success, just prepare your code handle multiple result sets because you'll need one for the success criteria and one for the data returned. If your DBA breaks down in tears over this, time to get a new one.
You can do what you're proposing. It's just not the common method. Result sets are a bit more costly in terms of network traffic. That's why using the return value or output parameters is preferred. When you call this procedure through ADO.NET, or whatever you're using, the call to the procedure will expect a zero or non-zero return value. You can also add the result set, but the return value is going to be there and be part of the process, so why not use it? You can test this in TSQL too:
DECLARE @Result tinyint
EXEC @Result = spr_MyProc
SELECT @return
Note, the SELECT statement here is just to show that a value was returned. It's not meant as a result set. Instead, you'd do something like this:
DECLARE @result tinyint
EXEC @result = spr_MyProc
IF @result 0
begin
exec spr_FailedProc
end
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
June 22, 2009 at 4:05 am
Thanks Grant. I've been pulled onto a different project, I'll get back to you about this.
June 24, 2009 at 11:52 am
Well, in a "true" relational model, there is no need to do a @@IDENTITY or SCOPE_IDENTITY(), therefore it is not reliable at all. Surrogate identifiers are not valid keys, it has none of the relational properties a key would have.
Do you really prefix all your tables/stored procedures? Do you know that Microsoft conventions recommend against prefixing SPs with "sp_"?
This is set based instead of your sequential "one record at a time" procedure:
CREATE TABLE [users]
(
user_email dbo.email NOT NULL UNIQUE
);
CREATE PROCEDURE InsertOrUpdateUser
(
@user_email dbo.email -- User Defined Type so you do not repeat everywhere
)
AS BEGIN
MERGE dbo.[users] AS a
USING(SELECT @user_emaill) AS b(user_email)
ON (a.user_email = b.user_email)
WHEN MATCHED THEN
UPDATE SET [column list to update]
WHEN NOT MATCHED THEN
INSERT (user_email, [column list to insert])
VALUES(@user_email, [column list to insert]);
END
Viewing 8 posts - 1 through 7 (of 7 total)
You must be logged in to reply to this topic. Login to reply