In my last post in this series,
I
covered the use of "bad" characters in entity names, such as spaces
or dashes. In this post I will talk about using RETURN and OUTPUT inappropriately.
Jamie Thomson touched on part of this pet peeve in response to one of the other posts in this series. So let me ask, do you see anything wrong with this procedure?
CREATE PROCEDURE dbo.foo
AS
BEGIN
SET NOCOUNT ON;
DECLARE
@hr INT,
@rc INT;
EXEC @hr = dbo.sp_columns 'dbo.bar';
SELECT @rc = @@ROWCOUNT;
SELECT hr = @hr;
RETURN (@rc);
END
GO
|
The title of this post kind of gives it away, but I thought it would fun to ask anyway. Here is what I see wrong:
- SELECT should be used for returning resultsets, not scalars. This procedure uses a SELECT statement to return a single value to the client. This is inefficient because most applications will have to prepare additional objects (typically referred to as "recordsets") and other support in order to consume the result. While it is certainly valid syntax to use SELECT to return scalar values, this does not need to be common in production code. This is the kind of thing that can make it slightly harder for high-end applications to scale.
While not definitive proof that this is bad, and while I use SELECT for multiple variable assignment, IIRC the standard does not allow SELECT without FROM. And in the 26 SELECT examples in Books Online, not one of them references scalars, at least at the time of writing.
- RETURN is for exiting the procedure, and is intended for returning error / status codes, not data. Do not use this to send back to the caller the latest IDENTITY value generated or the row count from an operation. Note that you are restricted to using integer-based data anyway, since you cannot change the data type of the value passed back by RETURN(). For more information, see the topic "Returning Data by Using a Return Code" in Books Online.
- OUTPUT is designed for returning scalar values (oh, and cursors, but who does that?). In the example above, we are sending two scalar values back to the caller (though, in fact, one should be a RETURN value), yet we are not using an OUTPUT parameter at all! For more information, see the topic "Returning Data by Using OUTPUT Parameters" in Books Online. As an aside, I suggest always using the full word OUTPUT; you may be tempted to just type OUT but personally I find this lazy shorthand that could prove troublesome later - for example, if you have to search your codebase for all uses of OUTPUT parameters.
Yes, using OUTPUT makes it a little more complex to develop and debug stored procedures, since you have to declare your output parameters up front. I am not against using SELECT for scalars while developing. But there is no reason to deploy them that way. You can even use a @debug parameter to switch the methodology depending on the scenario, e.g.:
CREATE PROCEDURE dbo.foo
@debug BIT = 0,
@rc INT = NULL OUTPUT
AS
BEGIN
SET NOCOUNT ON;
DECLARE @hr INT;
EXEC @hr = dbo.sp_columns 'dbo.bar';
SET @rc = @@ROWCOUNT;
IF @debug = 1
BEGIN
SELECT hr = @hr, rc = @rc;
END
RETURN (@hr);
END
GO
|
Now you can call this stored procedure with or without specifying the OUTPUT parameter up front. (I use this @debug technique for a lot of debugging elements, including cases where using the flag can change the plan - in the normal case this doesn't really hurt anything because the production plan is the one that is in the cache 99.99% of the time.)
-- debug mode:
EXEC dbo.foo @debug = 1;
-- normal operation:
DECLARE
@rc INT,
@hr INT;
EXEC @hr = dbo.foo @rc = @rc OUTPUT;
PRINT @hr;
PRINT @rc;
|
You'll notice that I laced the above commentary with several links to Books Online topics. I highly recommend becoming very familiar with when and where you should use RETURN, OUTPUT and SELECT to return data from a stored procedure.
I am working on a series of "Bad habits to kick" articles, in an
effort to motivate people to drop some of the things that I hate to see
when I inherit code. Up next: using SELECT * / omitting the column list.