August 20, 2009 at 12:27 pm
I'm not a programmer anymore - I was a long time ago ... I know my sql is old but I've been assigned the task of modifying a site and have to go with what I knew ....
My problem in the below code is the use of @mySqlStrOrg inside the select statement. I just want to dynamically build a static string inside the sp and then use it later in that same sp ... if that makes any sense at all ...
ALTER proc dbo.reportCreatorPage1Top10Cats
@MyDateIntervalStart smalldatetime = '',
@MyDateIntervalEnd smalldatetime = '',
@MyClientId int = null,
@MyDivision varchar(10) = '',
@MyDepartment varchar(10) = ''
as
begin
/* + + + + + + + + + + + + + + + + + + + + + + + + + + + */
/* + + + + + + + + Create Temp Table + + + + + + + + + */
CREATE TABLE #tempReportTbl1
(
rownr int,
catname varchar(50),
quan int,
totalresponseM int,
totalresponseH numeric(8,2),
avgrestimeM int,
avgrestimeH numeric(8,2)
)
/* + + + + + + + + + + + + + + + + + + + + + + + + + + + */
/* + + + + + + + + extra query params + + + + + + + + + + + + + */
declare @mySqlStrOrg as varchar(100)
set @mySqlStrOrg = ''
if @MyDivision = "abc"
begin
if @MyDepartment = "all"
/* all departments = unrestricted */
begin
set @mySqlStrOrg = ' and divisionunit.division_id = 1 '
end
end
else IF @MyDivision = "def"
begin
if @MyDepartment = "all"
/* all departments = unrestricted */
begin
set @mySqlStrOrg = ' and divisionunit.division_id = 2 '
end
end
else IF @MyDivision = "none"
Begin
/* default value from iis */
/* we are doing site only */
set @mySqlStrOrg = ' '
end
else IF @MyDepartment = "none"
Begin
/* default value from iis */
/* we are doing site only */
set @mySqlStrOrg = ' '
end
else
begin
set @mySqlStrOrg = ' and departmentunit.departmentunit_id = @MyDepartment '
end
/* + + + + + + + + + + + + + + + + + + + + + + + + + + + */
/* + + + + + + + + Fill table + + + + + + + + + + + + + */
/* + + + + + + Closed inquiries only + + + + + + + + + */
insert into #tempReportTbl1(catname, quan, totalresponseM, totalresponseH)
select top 10 category_name, count(*) as Quantity, sum(minutes_response), sum((minutes_response/60.00))
from inquiry, type, category, team, departmentunit, inquiry_work, department, divisionunit
where inquiry.type_id = type.type_id
and type.category_id = category.category_id
and category.team_cat = team.team_id
and team.departmentunit_id = departmentunit.departmentunit_id
and departmentunit.department_id = department.department_id
and department.divisionunit_id = divisionunit.divisionunit_id
and inquiry.inquiry_id = inquiry_work.inquiry_id
@mySqlStrOrg /*<= There is my problem */
and client_id = @MyClientId
and status_id = 2
and inquiry.log_date > @MyDateIntervalStart
and inquiry.log_date < @MyDateIntervalEnd
group by category_name order by Quantity Desc
[/code]
... plus much more but that's irrelevant at this time i guess
I do so appreciate some help. I've tried many variations. This should be so simple ... tried '+@mySqlStrOrg+', tried nvarchar, tried N'' .. possibly tried incorrectly .. ?
Thanx if you can help 🙂
August 20, 2009 at 12:31 pm
I would like to help but I'm not sure I understand your problem.
What results are you getting currently? Error messages? Null results? Unexpected obscenities appearing in the middle of the string? 😉
Work with us just a little bit and we'll get you an answer.
__________________________________________________
Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills
August 20, 2009 at 12:35 pm
Okay... I see where you are going wrong. You can't just use a variable to build code and stick it in the middle of code like that, unless you are building ALL the code dynamically and executing it using EXEC or the stored proc "sp_executeSQL". (Although you can test a column against the value of a variable.)
Here is a great article you should read. Holler back here if you have questions after.
http://sqlinthewild.co.za/index.php/2009/03/19/catch-all-queries/
__________________________________________________
Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills
August 20, 2009 at 12:39 pm
setup the entire statement as a string and concatenate your derived string into the statement. so, the following would be after your if statement to piece together the string:
declare @sql varchar(5000)
select @sql = 'insert into #tempReportTbl1(catname, quan, totalresponseM, totalresponseH)
select top 10 category_name, count(*) as Quantity, sum(minutes_response), sum((minutes_response/60.00))
from inquiry, type, category, team, departmentunit, inquiry_work, department, divisionunit
where inquiry.type_id = type.type_id
and type.category_id = category.category_id
and category.team_cat = team.team_id
and team.departmentunit_id = departmentunit.departmentunit_id
and departmentunit.department_id = department.department_id
and department.divisionunit_id = divisionunit.divisionunit_id
and inquiry.inquiry_id = inquiry_work.inquiry_id '
+ @mySqlStrOrg
+ '
and client_id = ' + @MyClientId + '
and status_id = 2
and inquiry.log_date > ' + @MyDateIntervalStart + '
and inquiry.log_date < ' + @MyDateIntervalEnd + '
group by category_name order by Quantity Desc '
exec (@sql)
HTH
August 20, 2009 at 12:50 pm
Thanx Bob - I'll check that out now ...
If I use
@mySqlStrOrg
in the select
sql says
Line 90: Incorrect syntax near '@mySqlStrOrg'.
if I use
+ @mySqlStrOrg
in the select it works as long as all conditions result in an empty string - ''
but If a condition is true and the string builds -
sql says
Syntax error converting the varchar value ' and divisionunit.division_id = 1 ' to a column of data type int.
I have no idea where he's getting that ... so I try
& @mySqlStrOrg
and I get the same msg ...
Thanx Chuck -
I tried something similiar before and couldn't get it to work ...
I get (with yours) and got (before) this error:
Message="Syntax error converting the varchar value 'insert into #tempReportTbl1(catname, quan, totalresponseM, totalresponseH)
select top 10 category_name, count(*) as Quantity, sum(minutes_response), sum((minutes_response/60.00))
from inquiry, type, category, team, departmentunit, inquiry_work, department, divisionunit
where inquiry.type_id = type.type_id
and type.category_id = category.category_i..."
Number=245
I tried some sort of nvarchar and N' variant but couldn't quite get it ...
August 20, 2009 at 12:53 pm
Nick, here is another article by Gail on the topic of SQL injection, which is a hacking technique used against SQL Servers. I'm including it because you really need to be aware of the security issues involved with doing dynamic SQL.
http://sqlinthewild.co.za/index.php/2009/04/03/dynamic-sql-and-sql-injection/
__________________________________________________
Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills
August 20, 2009 at 1:08 pm
Thanx again Bob -
in the first article ...
EXEC sp_executesql @sSQL, N'@_Product int, @_OrderID int, @_TransactionType char(1), @_Qty int',
@_Product = @Product, @_OrderID = @OrderID, @_TransactionType = @TransactionType, @_Qty = @Qty
I don't fully understand what this is .. ? I understand EXEC sp_executesql @sSQL but not the rest ... and how I would write it for mine .. ?
Also - thanx for the heads up about security ... this is a fully an intranet app and if someone was actaully that malicious then they could just have it ... guess I could have server admins rollback to day -1 without a broken heart ...
Chuck - do you have any idea what it's trying to convert the varchar to? The message cuts off ...
I tried
declare @sql nvarchar(4000)
select @sql = N'insert into #tempReportTbl1(catname, quan, totalresponseM, totalresponseH)
select top 10 category_name, count(*) as Quantity, sum(minutes_response), sum((minutes_response/60.00))
from inquiry, type, category, team, departmentunit, inquiry_work, department, divisionunit
where inquiry.type_id = type.type_id
and type.category_id = category.category_id
and category.team_cat = team.team_id
and team.departmentunit_id = departmentunit.departmentunit_id
and departmentunit.department_id = department.department_id
and department.divisionunit_id = divisionunit.divisionunit_id
and inquiry.inquiry_id = inquiry_work.inquiry_id '
+ @mySqlStrOrg + N'
and client_id = ' + @MyClientId + N'
and status_id = 2
and inquiry.log_date > ' + @MyDateIntervalStart + N'
and inquiry.log_date < ' + @MyDateIntervalEnd + N'
group by category_name order by Quantity Desc '
exec (@sql)
as well, but getthe same msg a convert nvarchar to .. ?
August 20, 2009 at 1:16 pm
the issue is around your other variables being included in the concatenation. so, instead of just saying:
and client_id = ' + @MyClientId + '
change it to read (I am guessing on size of 5 here) :
and client_id = ' + cast(@MyClientId as varchar(5)) + '
you will need to do that also for the datetime variables. the string needs to make sure that everything pieced together is of the varchar type.
also, i would test out the pieced together statement by commenting out the exec (@sql) line and printing it first.
--exec (@sql)
print @sql
then execute the procedure and ensure that sql that is printed is what you want to execute. if so, uncomment the exec line and comment out the print line and then execute.
the sql injection article is a good point and you will want to do a little checking around the @MyDepartment and @MyDivision variables to make sure you do not get any invalid statements included in the string.
August 20, 2009 at 1:56 pm
Nick, what you are seeing is an example of "parameterized" dynamic SQL. Instead of building the entire string, you can actually pass values into it as parameters. This is one of the things you do to help defeat SQL injection attempts. Here is a simple example.
declare @db1 nvarchar(100)
declare @db2 nvarchar(100)
declare @SQL nvarchar(4000)
set @db1 = 'Master'
-- set @db1 = 'Master''); select ''Hi Mom''; --'
set @db2 = 'Model'
-- parameterized and secure
set @SQL = 'Select * from sys.databases where name in (@parm1,@parm2)'
exec sp_executesql
@SQL, -- dynamic SQL string to be executed
N'@parm1 nvarchar(100), @parm2 nvarchar(100)', -- "declarations" of parameters for @SQL
@parm1 = @db1, @parm2 = @db2 -- assignment of values to parameters in @SQL string
-- concatenated and vulnerable
set @SQL = 'SELECT * from sys.databases where name in ('''+@db1+''','''+@db2+''')'
print @sql
EXEC (@sql)
In the code above, values for @DB1 and @DB2 would typically be entered by a user through their web interface.
Notice that the first example expressly sets the value of of the @SQL string to be executed without doing any string concatentations. The values of @db1 is passed to the string in place of @parm1 and the value of @db2 is passed in place of @parm2. By contrast, the second example builds the entire string and uses EXEC to execute it.
Run the above code and you will see the results are the same, BUT if you uncomment the line that sets @DB1 to a string containing the "Hi Mom" code, you will see a big difference. In the second example, I was able to insert, not just values, but executable SQL statements into the @SQL string.
If the user interface allows me to type in long enough strings, I can do more than run a SELECT to say "Hi Mom". I can write more complicated code to query (or update, or insert into, or delete) other tables. This is the danger of SQL injection and the reason why writing dynamic SQL to be parameterized is well worth the trouble.
Edited to add:
I understand that you are probably in a crunch and that in your example, there may be no input coming from a user that would enable an SQL injection attack. However, the next time around there might be. If you are getting into coding dynamic SQL, it's better to know about this up front and start learning how to do it right. If you are learning to do catch-all or wildcard queries, you need to know about the performance differences produced by different approaches. This is why I encourage you to read and understand those articles.
__________________________________________________
Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills
August 20, 2009 at 2:38 pm
thanx guys, I'll play a bit and see what I get - just got an error because when I cast the date it changes the format from yyyy-mm-dd hh:mm:ss to MMM dd yyyy hh:mm tt ... wierd
Think I'll change the smalldatetime spec to varchar and see whatzup ...
August 20, 2009 at 2:47 pm
Hey Nick,
CONVERT() gives you more control over the format of the output string than CAST().
__________________________________________________
Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills
August 24, 2009 at 1:00 am
Hey guys, Thanks for all the help. It turned out great!
Viewing 12 posts - 1 through 11 (of 11 total)
You must be logged in to reply to this topic. Login to reply