Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

 All Forums
 SQL Server 2000 Forums
 Transact-SQL (2000)
 IF Input Parameter IS NULL

Author  Topic 

Mr Fett
Starting Member

28 Posts

Posted - 2010-07-19 : 12:14:50
Hi all,

Apologies for the simple newbie question (I'm no doubt missing something simple but I've tried for hours!), I'm generating my first Dynamic SQL stored procedure and am getting an error saying

Must declare the variable '@locationID', when I move that line the error changes to the next input parameter (Must declare the variable '@yachtTypeID').

I've tried declaring an alternative variable called "@declaredLocationID" and doing SET @declaredLocationID = @locationID and then using that in the SQL statement but it then just tells me that "@declaredLocationID" isn't declared.

Here's my procedure:



CREATE PROCEDURE dbo.fetchYachts

@locationID int,
@yachtTypeID int,
@orderBy varchar (80)

AS

DECLARE @SQLStatement varchar(1024)

SET @SQLStatement = 'SELECT * FROM YACHTS_yachts INNER JOIN YACHTS_makes ON YACHTS_yachts.makeID = YACHTS_makes.makeID INNER JOIN YACHTS_yachtTypes ON YACHTS_yachts.yachtTypeID = YACHTS_yachtTypes.yachtTypeID INNER JOIN YACHTS_rates ON YACHTS_yachts.yachtID = YACHTS_rates.yachtID INNER JOIN YACHTS_yachtLocationLink ON YACHTS_yachts.yachtID = YACHTS_yachtLocationLink.yachtID WHERE YACHTS_yachtLocationLink.locationID = @locationID AND (YACHTS_rates.rateYear = 2010)'

IF @yachtTypeID IS NOT NULL SET @SQLStatement = @SQLStatement + ' AND (YACHTS_yachts.yachtTypeID = @yachtTypeID ) '

SET @SQLStatement = @SQLStatement + ' ORDER BY @orderBy '


EXEC(@SQLStatement)
GO


Thanks in advance for any help!

Bob

Kristen
Test

22859 Posts

Posted - 2010-07-19 : 12:28:15
IF @yachtTypeID IS NOT NULL SET @SQLStatement = @SQLStatement + ' AND (YACHTS_yachts.yachtTypeID = ' + CONVERT(varchar(20), @yachtTypeID) + ' ) '

But lots of other things to worry about ...

Risk of SQL Injection, permissions required on underlying tables, performance issues.

Would be much better to use sp_ExecuteSQL instead of EXEC for this type of dynamic SQL
Go to Top of Page

Kristen
Test

22859 Posts

Posted - 2010-07-19 : 12:33:47
P.S. Don't use SELECT * - name the specific columns that you need (otherwise if you add some huge TEXT / IMAGE column in the future that will be rerieved, and use bandwidth getting to your application, and THEN be ignored by the application ...)

For your query I think this would do (no dynamic SQL) provided that the number of possible values for @orderBy is "modest":

SELECT Col1, Col2, ...
FROM YACHTS_yachts
INNER JOIN YACHTS_makes
ON YACHTS_yachts.makeID = YACHTS_makes.makeID
INNER JOIN YACHTS_yachtTypes
ON YACHTS_yachts.yachtTypeID = YACHTS_yachtTypes.yachtTypeID
INNER JOIN YACHTS_rates
ON YACHTS_yachts.yachtID = YACHTS_rates.yachtID #
INNER JOIN YACHTS_yachtLocationLink
ON YACHTS_yachts.yachtID = YACHTS_yachtLocationLink.yachtID
WHERE YACHTS_yachtLocationLink.locationID = @locationID
AND YACHTS_rates.rateYear = 2010
AND (@yachtTypeID IS NULL OR YACHTS_yachts.yachtTypeID = @yachtTypeID)
ORDER BY CASE WHEN @orderBy = 'Col1' THEN Col1 ELSE NULL END,
CASE WHEN @orderBy = 'Col2' THEN Col2 ELSE NULL END,
...

Go to Top of Page

Mr Fett
Starting Member

28 Posts

Posted - 2010-07-19 : 13:11:44
Hi Kristen,

That's wonderful thank you! My actual query already lists the column names (but it's rather large so I minimized it for pasting here) and all input is very strictly sanitized at the code level so no risk of SQL injection. The number of orderby options is indeed very modest so I'll use your later example. In the spirit of learning (and only if you have time), I've got a couple of other questions:

1) What is the difference between ExecuteSQL and EXEC?
2) What's the deal with converting the yachtTypeID to a string for a numeric comparison?

Thanks again for taking the time to help and I understand if you are unable to help with the questions!

Regards

Bob
Go to Top of Page

Kristen
Test

22859 Posts

Posted - 2010-07-19 : 14:39:53
1. sp_ExecuteSQL uses a parametrised query, and the query is cached, so will be as efficient as a stored procedure. EXEC will cache the query too, but the exact same query (i.e. including parameter substitutions) is cached, so unlikely to be reused, or rarely so.

2. You are creating a dynamic SQL query. If you say

EXEC ('SELECT * FROM MyTable WHERE MyColumn = @PARAMETER')

then @PARAMETER is not in scope.

So you have to say:


EXEC ('SELECT * FROM MyTable WHERE MyColumn = 10')

and thus to construct that in your @SQLStatement string you have to replace the @PARAMETER in the string with the actual value you are comparing again.

But with sp_ExecuteSQL you could do use @PARAMETER in your dynamic SQL query - because the essence of sp_ExcteuSQL is that you can pass parameters to sp_ExectueSQL itself, and then they are in scope / available to such queries

"all input is very strictly sanitized at the code level so no risk of SQL injection"

Sorry, but that is extremely unlikely to be the case. The only way that I know of to protect against SQL Injection is to assume that any user-supplied data parameter is a risk EVERY TIME it is used in dynamic SQL.

If you use user-supplied-data in parametrised queries then you are safe, but the moment you concatenate a string to make a SQL statement, for dynamic execution, with some user-supplied data you are at risk (unless you encapsulate the user-supplied-data in quotes, and double-up any embedded quotes - every single time ... and without fail ...).
Go to Top of Page

Mr Fett
Starting Member

28 Posts

Posted - 2010-07-20 : 09:00:29
Hi Kristen,

Thanks for the help and tips - it's much appreciated!!!

Regarding the sanitized input - trust me on this - the technique I'm using has rescued websites that have actually been under attack from SQL injections when I stepped in. While the input is 'user based' (passed from a query string), the only way this data gets to the SQL query is via a sanitizing function that uses regex to strip out all non-numeric characters. The only error a user can cause (deliberately or through ignorance) is an out-of-bounds error due to a number being passed that is higher than INT can handle (so if someone attempted to pass " AND 1 = 1 (DELETE * FROM USERS_Users) ", all that gets through is '11'). The order by code (which is a string) isn't user input - it's hard coded and is impossible for the user to manipulate.

Thanks again for taking the time to explain - very good of you!

Bob
Go to Top of Page

Mr Fett
Starting Member

28 Posts

Posted - 2010-07-20 : 11:36:54
Hi Kristen,

I tried the example and it worked a treat, one thing that I've hit a problem now however is the order direction. I was passing it along with the column as one string but using your example that wouldn't work (as it must match a column name). I have therefore tried to expand on your method by adding another parameter to the stored procedure called @orderDirection and adding these lines:

CASE WHEN @orderDirection = 'ASC' THEN ASC ELSE NULL END,
CASE WHEN @orderDirection = 'DESC' THEN DESC ELSE NULL END

But I get an error even trying to save the stored procedure saying "incorrect synatax near 'ASC'"


I was also wondering if there was any specific reason you had a CASE constuct for every possibility as opposed to something like this:


CASE @orderBy
WHEN 'yachtName' THEN yachtName
WHEN 'boatYear' THEN boatYear
WHEN 'maxPax' THEN maxPax
END

.. as I'm assuming that with multiple case statements, each one is queried even if one preceding it was already found to be true (so a performance drop).

Obviously I'm just guessing as SQL syntax is obviously very different to what I'm used to! (javascript, vbscript, php, etc).

Thanks again for your time and energy on this!

Bob
Go to Top of Page

Kristen
Test

22859 Posts

Posted - 2010-07-20 : 13:09:01
[code]
ORDER BY CASE WHEN @orderBy = 'Col1' AND @orderDirection = 'ASC' THEN Col1 ELSE NULL END ASC,
CASE WHEN @orderBy = 'Col1' AND @orderDirection = 'DESC' THEN Col1 ELSE NULL END DESC,
CASE WHEN @orderBy = 'Col2' AND @orderDirection = 'ASC' THEN Col2 ELSE NULL END ASC,
...
[/code]
gets pretty tedious pretty fast though ...
[code]
CASE @orderBy
WHEN 'yachtName' THEN yachtName
WHEN 'boatYear' THEN boatYear
WHEN 'maxPax' THEN maxPax
END
[/code]
Yeah, that's fine provided they are all the same data type. I would not worry about performance - although worth looking at whether the cached query plan may be suboptimal for some cases - in which case parametrised dynamic SQL would be better as each variant would be separately cached.

Not sure RegEx that only let through digits would be much use to me ... but if that's all you need for your queries its a solution. Can't say I would trust it though, so I'll still be in favour of "insulating" the dynamic SQL queries too.
Go to Top of Page
   

- Advertisement -