1

I have two tables tbl_Products and tbl_Brands, both are joined on BrandId.

I have a stored procedure which should return all the products belong to the brand ids passed to it as parameter.

My code is as follows.

create proc Sp_ReturnPrdoucts
    @BrandIds varchar(500) = '6,7,8'
AS
BEGIN
    SELECT * 
    FROM tbl_Products as p 
    JOIN tbl_Brands b ON p.ProductBrandId = b.BrandId 
    WHERE b.BrandId IN (@BrandIds)
END

But this is giving error as BrandId is INT and @BrandIds is VARCHAR

When I hard code it this way as follows it works fine and returns the desired data from db ..

create proc Sp_ReturnPrdoucts
    @BrandIds varchar(500) = '6,7,8'
AS
BEGIN
    SELECT * 
    FROM tbl_Products AS p 
    JOIN tbl_Brands b ON p.ProductBrandId = b.BrandId 
    WHERE b.BrandId IN (6,7,8)
END

Any help :)

8

2 Answers 2

4

If possible, don't use varchar for this kind of things, use a table valued parameter instead.

To use a tabled value parameter you should first declare a user defined table type:

CREATE TYPE IntList As Table
(
    IntValue int
)

Then change your stored procedure to accept this variable instead of the nvarchar:

create proc Sp_ReturnPrdoucts
    @BrandIds dbo.IntList readonly -- Note: readonly is a must!
AS
BEGIN

SELECT * 
FROM tbl_Products as p 
join tbl_Brands b on p.ProductBrandId=b.BrandId 
join @BrandIds ON(b.BrandId = IntValue)

END

The problem is that the IN() operator expects a list of variables separated by commas, while you provide a single variable that it's value is a comma separated string.

If you can't use a table valued parameter, you can use a string spliting function in sql to convert the value of the varchar to a table of ints. there are many splitters out there, I would recommend reading this article before picking one.

1
  • Nice and solid solution!
    – SQL Police
    Jun 13, 2015 at 10:20
2

Another alternative is to use 'indirection' (as I've always called it)

You can then do..

create proc Sp_ReturnPrdoucts
@BrandIds varchar(500) = '6,7,8'
AS
BEGIN
    if (isnumeric(replace(@BrandIds,',',''))=1) 
    begin
        exec('SELECT * FROM tbl_Products as p join tbl_Brands b on p.ProductBrandId=b.BrandId WHERE b.BrandId IN ('+@BrandIds+')')
    end
END

This way the select statement is built as a string, then executed.

I've now added validation to ensure that the string being passed in is purely numeric (after removing all the commas)

9
  • Downvote reason: Dynamic sql comes with a price, both in sql injection vulnerability and in performance. I would use it only as a last resort. Jun 13, 2015 at 10:00
  • Downvote? It's a valid solution, and each answer has its own advantages and disadvantages (this one is the shortest and easiest to read). There are ways to protect against sql injection, the performance hit is tiny, and only noticeable if you're running this SP repeatedly.
    – Rich S
    Jun 13, 2015 at 10:06
  • I would not have downvoted your answer if I thought that dynamic sql is a valid solution to this question. moreover, though it is possible to protect against sql injection, your answer does not even mention this problem, nor does your suggested solution is protedted from it in any way. Jun 13, 2015 at 10:09
  • In the way you write it here, it is totally vulnerable for SQL injection, because you don't check the parameters. @ZoharPeled I agree, DynSQL can be open for SQL Injection. However, performance can massively be improved due to DynSQL, in some situations.
    – SQL Police
    Jun 13, 2015 at 10:18
  • I've now edited to ensure that the string passed in doesn't contain anything other than commas and numbers.
    – Rich S
    Jun 13, 2015 at 10:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.