building string in sp (as varchar) for use in select

  • 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

    @MyDateIntervalStartsmalldatetime = '',

    @MyDateIntervalEndsmalldatetime = '',

    @MyClientIdint = null,

    @MyDivisionvarchar(10) = '',

    @MyDepartmentvarchar(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 🙂

  • 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

  • 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

  • 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

  • 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 ...

  • 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

  • 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 .. ?

  • 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.

  • 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

  • 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 ...

  • 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

  • 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