Dynamic SQL Query Not Fetching Records based on where Not In condition
-
Hello All ... Hope You all are Good .. Actually i am facing problem while working on dynamic sql with stored procedure. I am writing a simple stored procedure using popular "Northwind"' 's employees table. This stored procedure simply fetching the records from table using "employeeid" column based on where condition. Everything working ok if am taking employeeid column as varchar in stored procedures's input parameter. Problem arising when i am declaring it as integer. Though i have tried convert function to but that too isnt working .. It seems i am Lost in the web of comma's(') which are most annoying thing while working with dynamic sql .So, please someone give me the solution of my impending problem and also give me some astute insight on how to work on comma's in case of varchar , integer , date and about + symbol in queries.
Thanks
ALTER PROCEDURE [dbo].[UDPR_employeeview]@employeeid integer
As
Begin
Declare @SQL varchar (Max)
Set @SQL ='select title, titleofcourtesy from employees Where employeeid Not in (' + @employeeid + ')'--SET @SQL='select title, titleofcourtesy from employees Where employeeid Not in (CONVERT(VARCHAR(24), ' + @employeeid + ')'
Set @SQL=@SQL
SELECT @SQL=@SQLPrint (@SQL)
Execute(@SQL)
End -
this article may help you
https://www.codeproject.com/Articles/20815/Building-Dynamic-SQL-In-a-Stored-Procedure
________________________________________________________________
you can lead a user to data....but you cannot make them think
and remember....every day is a school day - J Livingston SQL - Saturday, July 22, 2017 4:03 AM
Thanks but ..I have already seen this article ... But that too isn't solving my problem ... My code is working fine when i am using Varchar variable as i told in my post .. you can rather go through my code and give a solution if you want to.rather then referencing anything else ..As you can well understand that how things work in programming .. !!
- sunilchand1234 - Saturday, July 22, 2017 4:09 AM
I am not sure why the article didnt help you because the first example is very close to what you want
that said
DECLARE @employeeid INT= 100;
DECLARE @SQL VARCHAR(MAX);-- this corrects what you are trying
SET @SQL='SELECT title, titleofcourtesy FROM employees WHERE employeeid NOT IN ( ' + CONVERT(VARCHAR(24), @employeeid )+')';
PRINT(@SQL);
-- or
SET @SQL = 'SELECT title, titleofcourtesy FROM employees WHERE employeeid NOT IN ( ' + CAST(@employeeid AS VARCHAR(10)) +')';
PRINT(@SQL);
-- or maybe this
SET @SQL = 'SELECT title, titleofcourtesy FROM employees WHERE employeeid <> ' + CAST(@employeeid AS VARCHAR(10)) ;
PRINT(@SQL);________________________________________________________________
you can lead a user to data....but you cannot make them think
and remember....every day is a school day - sunilchand1234 - Saturday, July 22, 2017 12:21 AM
________________________________________________________________
you can lead a user to data....but you cannot make them think
and remember....every day is a school day - J Livingston SQL - Saturday, July 22, 2017 4:32 AM
It's working now... Thanks a Ton ....
-
This code is wildly open to SQL injection. If you don't know what I'm talking about, please google it.
Scratch that.
This code has no reason to be dynamic. It also has no reason to use NOT IN.
It just needs a simple approach:SELECT title, titleofcourtesy FROM employees WHERE employeeid <> @employeeid;Now, if the code is more complex that what you have shown in here, unscratch the first part and read about SQL injection.
- Luis Cazares - Monday, July 24, 2017 6:46 AM
How ?? can you tell us more about SQL Injection in more details using the example or the same you have mentioned . It will be really helpful !
- Luis Cazares - Monday, July 24, 2017 6:46 AM
The more I think about this, the more convinced I become that the intent of having NOT IN as part of the query was that someone was going to provide a list of employee ID values, and that the idea of using dynamic SQL was predicated on that being a comma delimited list of integers. Of course, that's a particularly bad idea given the potential for SQL injection, for any kind of site that faces the world wide web, as just a few extra lines of code provided in an input line could then be a serious problem. There's a fairly straightforward way to turn the ID values into a table and have a stored procedure use a parameter that is a table, which would then eliminate the need for any dynamic SQL.
Steve (aka sgmunson) 🙂 🙂 🙂
Rent Servers for Income (picks and shovels strategy) - sgmunson - Wednesday, July 26, 2017 9:30 AM
Still doesn't need to be dynamic:
DECLARE @EmpIds VARCHAR(8000) = '123,345,567,2356,790,8576';SELECT
title
, titleofcourtesy
FROM
employees
WHERE
employeeid NOT IN (SELECT CAST([dsk].[Item] AS INT)FROM dbo.[DelimitedSplit8K](@EmpIds, ',') AS [dsk] ); - sunilchand1234 - Wednesday, July 26, 2017 6:40 AM
Let's try to go step by step. I'll create a table to have something to test.
IF OBJECT_ID('Employees') IS NOT NULL
DROP TABLE Employees;CREATE TABLE employees(
employeeid int,
title varchar(20),
titleofcourtesy varchar(20)
);
INSERT INTO employees VALUES(1, 'King', 'Your Majesty'), (2, 'Queen', 'Your Majesty'), (3, 'Judge', 'Your Honor')
Now we simplify your procedure:
CREATE PROCEDURE [dbo].[UDPR_employeeview]
@employeeid int
As
Begin
Declare @SQL varchar (Max)
Set @SQL ='select title, titleofcourtesy from employees Where employeeid Not in (' + @employeeid + ')'Print (@SQL)
Execute(@SQL)
End
GO
If we run the procedure, we get a conversion error. To fix this error, we use CONVERT when concatenating the parameter.
CREATE PROCEDURE [dbo].[UDPR_employeeview]
@employeeid int
As
Begin
Declare @SQL varchar (Max)
Set @SQL ='select title, titleofcourtesy from employees Where employeeid Not in (' + CONVERT( varchar(15), @employeeid) + ')'Print (@SQL)
Execute(@SQL)
End
GO
Now, we can call it and get some results.
EXEC [dbo].[UDPR_employeeview] 1;
Right now, there's no SQL injection problem. But by using NOT IN, it's implied that you want to use multiple values. Let's try the following:
EXEC [dbo].[UDPR_employeeview] 1,2
We get the following results:
Msg 8144, Level 16, State 2, Procedure UDPR_employeeview, Line 23
Procedure or function UDPR_employeeview has too many arguments specified.
What about this?
EXEC [dbo].[UDPR_employeeview] '1,2'
Another errorMsg 8114, Level 16, State 5, Procedure UDPR_employeeview, Line 23
Error converting data type varchar to int.
Then, the problem is that we need to allow the parameter to allow a string with comma-delimited values. To allow enough values, we change the type to varchar(100) and remove the CONVERT() when concatenating the values.
CREATE PROCEDURE [dbo].[UDPR_employeeview]
@employeeid varchar(100)
As
Begin
Declare @SQL varchar (Max)
Set @SQL ='select title, titleofcourtesy from employees Where employeeid Not in (' + @employeeid + ')'Print (@SQL)
Execute(@SQL)
End
GO
We run the procedure again with one value and with 2.
EXEC [dbo].[UDPR_employeeview] 1
EXEC [dbo].[UDPR_employeeview] '1'
EXEC [dbo].[UDPR_employeeview] '1,2'
We got it working. But now, what would happen if someone wants to play around and sends some unexpected values?
EXEC [dbo].[UDPR_employeeview] 'NULL); TRUNCATE TABLE employees;--'
SELECT * FROM employees
Our table has been truncated!!!!!! :w00t::blink:
This is just a simple example. In reality, they won't try to simply truncate your table, they could get restricted information, drop the whole db, grant themselves full sysadmin access, etc.
To prevent this, you could follow Lynn's advice, use table-valued parameters or many other options available.
This does not means that you should never use Dynamic SQL. As with most things, it got its uses and safety measures that give great performance and functionality. I recommend that you read some more about this problem as there's many information online.
https://www.red-gate.com/simple-talk/sql/database-administration/sql-injection-how-it-works-and-how-to-thwart-it/
http://www.sommarskog.se/dynamic_sql.html#SQL_injection
https://technet.microsoft.com/en-us/library/ms161953(v=sql.105).aspx
http://bobby-tables.com/
-
And I forgot, be sure to test for NULLs functionality
EXEC [dbo].[UDPR_employeeview] '1,NULL' - Luis Cazares - Wednesday, July 26, 2017 12:45 PMsunilchand1234 - Wednesday, July 26, 2017 6:40 AM
Let's try to go step by step. I'll create a table to have something to test.
IF OBJECT_ID('Employees') IS NOT NULL
DROP TABLE Employees;CREATE TABLE employees(
employeeid int,
title varchar(20),
titleofcourtesy varchar(20)
);
INSERT INTO employees VALUES(1, 'King', 'Your Majesty'), (2, 'Queen', 'Your Majesty'), (3, 'Judge', 'Your Honor')
Now we simplify your procedure:
CREATE PROCEDURE [dbo].[UDPR_employeeview]
@employeeid int
As
Begin
Declare @SQL varchar (Max)
Set @SQL ='select title, titleofcourtesy from employees Where employeeid Not in (' + @employeeid + ')'Print (@SQL)
Execute(@SQL)
End
GO
If we run the procedure, we get a conversion error. To fix this error, we use CONVERT when concatenating the parameter.
CREATE PROCEDURE [dbo].[UDPR_employeeview]
@employeeid int
As
Begin
Declare @SQL varchar (Max)
Set @SQL ='select title, titleofcourtesy from employees Where employeeid Not in (' + CONVERT( varchar(15), @employeeid) + ')'Print (@SQL)
Execute(@SQL)
End
GO
Now, we can call it and get some results.
EXEC [dbo].[UDPR_employeeview] 1;
Right now, there's no SQL injection problem. But by using NOT IN, it's implied that you want to use multiple values. Let's try the following:
EXEC [dbo].[UDPR_employeeview] 1,2
We get the following results:
Msg 8144, Level 16, State 2, Procedure UDPR_employeeview, Line 23
Procedure or function UDPR_employeeview has too many arguments specified.
What about this?
EXEC [dbo].[UDPR_employeeview] '1,2'
Another errorMsg 8114, Level 16, State 5, Procedure UDPR_employeeview, Line 23
Error converting data type varchar to int.
Then, the problem is that we need to allow the parameter to allow a string with comma-delimited values. To allow enough values, we change the type to varchar(100) and remove the CONVERT() when concatenating the values.
CREATE PROCEDURE [dbo].[UDPR_employeeview]
@employeeid varchar(100)
As
Begin
Declare @SQL varchar (Max)
Set @SQL ='select title, titleofcourtesy from employees Where employeeid Not in (' + @employeeid + ')'Print (@SQL)
Execute(@SQL)
End
GO
We run the procedure again with one value and with 2.
EXEC [dbo].[UDPR_employeeview] 1
EXEC [dbo].[UDPR_employeeview] '1'
EXEC [dbo].[UDPR_employeeview] '1,2'
We got it working. But now, what would happen if someone wants to play around and sends some unexpected values?
EXEC [dbo].[UDPR_employeeview] 'NULL); TRUNCATE TABLE employees;--'
SELECT * FROM employees
Our table has been truncated!!!!!! :w00t::blink:
This is just a simple example. In reality, they won't try to simply truncate your table, they could get restricted information, drop the whole db, grant themselves full sysadmin access, etc.
To prevent this, you could follow Lynn's advice, use table-valued parameters or many other options available.
This does not means that you should never use Dynamic SQL. As with most things, it got its uses and safety measures that give great performance and functionality. I recommend that you read some more about this problem as there's many information online.
https://www.red-gate.com/simple-talk/sql/database-administration/sql-injection-how-it-works-and-how-to-thwart-it/
http://www.sommarskog.se/dynamic_sql.html#SQL_injection
https://technet.microsoft.com/en-us/library/ms161953(v=sql.105).aspx
http://bobby-tables.com/Ahh... !! Thanks .. It's been quite useful ! I will certainly study those links you have provided to prevent the problem of 'SQL Injection' .
-
You should also look at where you are using dynamic SQL and if it isn't really needed rewrite it without dynamic SQL.
Viewing 14 posts - 1 through 13 (of 13 total)
You must be logged in to reply to this topic. Login to reply