January 20, 2009 at 10:03 am
CREATE PROCEDURE sp_GetTasks
@colname nvarchar(50),
@colvalue int
AS
BEGIN
SELECT * FROM Tasks
WHERE @colname=@colvalue
END
The above simple proc returns an error in the title. I dont know what is wrong. Can anyone help?
Thanks,
January 20, 2009 at 10:10 am
When you want to use a variable in a SQL statement, you need to do this a bit different:
CREATE PROCEDURE sp_GetTasks
@colname nvarchar(50),
@colvalue int
AS
BEGIN
DECLARE @sql varchar(2000)
SELECT @sql = 'SELECT * FROM Tasks'
SELECT @sql = @sql +'WHERE '+ @colname + '=' + @colvalue
EXEC (@SQL)
END
I hope this does the work, let me know if you run into troubles.
Ronald HensbergenHelp us, help yourself... Post data so we can read and use it: http://www.sqlservercentral.com/articles/Best+Practices/61537/-------------------------------------------------------------------------2+2=5 for significant large values of 2
January 20, 2009 at 10:16 am
To do what you are trying to accomplish, you will need to use dynamic sql. Based solely on the code you provided I see it as very dangerous as it opens your system to SQL injection attacks.
First thing I'd suggest is rethink what you are trying to accomplish. You may want multiple stored procedures to query the table, one for each column that can be queried. Then a master stored procedure that then calls the appropriate stored procedure based on the data passed into the stored procedure.
If you really need to do it with one procedure, read BOL regarding dynamic sql, and then do a lot of research on protecting yourself from SQL injection attacks.
January 20, 2009 at 10:20 am
r.hensbergen (1/20/2009)
When you want to use a variable in a SQL statement, you need to do this a bit different:
CREATE PROCEDURE sp_GetTasks
@colname nvarchar(50),
@colvalue int
AS
BEGIN
DECLARE @sql varchar(2000)
SELECT @sql = 'SELECT * FROM Tasks'
SELECT @sql = @sql +'WHERE '+ @colname + '=' + @colvalue
EXEC (@SQL)
END
I hope this does the work, let me know if you run into troubles.
Just remember that the above is dangerous. Example:
exec sp_GetTasks '1 = 1;drop table Tasks --', 1
Guess what, I just dropped the Tasks table if I have the permissions to do so. Other things could also be done to delete or corrupt your data, and not just this one table.
January 20, 2009 at 11:26 am
Lynn, Thanks for your advice.
January 20, 2009 at 11:29 am
Ronald,
Your seggetion encountered with the same type of error saying...
Conversion failed when converting the nvarchar value 'SELECT * FROM Tasks WHERE colname=' to data type int
Any comments???
Thanks,
January 20, 2009 at 11:35 am
r.hensbergen (1/20/2009)
When you want to use a variable in a SQL statement, you need to do this a bit different:
CREATE PROCEDURE sp_GetTasks
@colname nvarchar(50),
@colvalue int
AS
BEGIN
DECLARE @sql varchar(2000)
SELECT @sql = 'SELECT * FROM Tasks'
SELECT @sql = @sql +'WHERE '+ @colname + '=' + @colvalue
EXEC (@SQL)
END
I hope this does the work, let me know if you run into troubles.
Before you run dynamic SQL like this, check against the system to make sure it's valid. One thing you should do is enclose the column name in square brackets, which will avoid SQL injection attacks. Another thing you should do is check that it's a valid column name by querying sys.columns.
Something like:
CREATE PROCEDURE sp_GetTasks
@colname nvarchar(50),
@colvalue int
AS
BEGIN
if not exists
(select *
from sys.columns
where name = @colname
and object_id = object_id(N'Tasks'))
begin
raiserror('Column requested does not exist', 16, 1)
return
end
DECLARE @sql varchar(2000)
SELECT @sql = 'SELECT * FROM Tasks'
SELECT @sql = @sql +'WHERE ['+ @colname + ']=' + @colvalue
EXEC (@SQL)
END
It's very important in dynamic SQL to make sure it's not going to allow injection.
If, with the modified query, someone uses "1=1;drop table Tasks--" as the column name, there won't be a column with that name, and it will error out. The brackets added to the second query are pretty much overkill in this case, but still useful as a habit.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
January 20, 2009 at 11:40 am
One last change to the code provided by GSquared:
CREATE PROCEDURE sp_GetTasks
@colname nvarchar(50),
@colvalue int
AS
BEGIN
if not exists
(select *
from sys.columns
where name = @colname
and object_id = object_id(N'Tasks'))
begin
raiserror('Column requested does not exist', 16, 1)
return
end
DECLARE @sql varchar(2000)
SELECT @sql = 'SELECT * FROM Tasks'
SELECT @sql = @sql +'WHERE ['+ @colname + ']=' + cast(@colvalue as varchar(10))
EXEC (@SQL)
END
January 20, 2009 at 11:51 am
GSquared,
Thanks much for your comprehensive advice. It is very helpful.
January 20, 2009 at 12:22 pm
Lynn,
What is the purpsoe of Cast for @colvalue. It is declared as an integer, now you changed to varchar.
Yes, it helps to solve the problem of conversion error above, but how? why? Can you explain please?
Thanks,
January 20, 2009 at 12:23 pm
You're welcome.
And Lynn's change is necessary. I just copy-and-pasted that part, didn't even look at it, but it will end up with an error because of the conversion of integer to string, unless you add the conversion step to it.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
January 20, 2009 at 12:56 pm
Yes, you are welcome also.
I have written enough dynamic sql that I've been bitten by that error many times. I have learned to look for it.
Viewing 12 posts - 1 through 11 (of 11 total)
You must be logged in to reply to this topic. Login to reply