1

I have my query below and getting error Must declare the scalar variable "@collectionId", even if I use [GetAllProductById] 6,0,0,0,'A122'

ALTER proc [dbo].[GetAllProductById] 
(
    @collectionId int,
    @GrandId int  ,
    @ParentId int ,
    @ChildId int,
    @dealerid varchar(50) 
)
As
Begin

Declare @sql as varchar(max)
 -- In case dealer is logged in ,then calculate the Discounted amount and return the same,
    -- else return Mrp and Our Price for all other customers

    IF  @collectionid<=0

        BEGIN
            IF @dealerid<>''
                BEGIN
                    SET @sql = '    Select Top(5) Product.Id,ProdImage,ProductCode, ProductName,MrpPrice, BasicPrice,ValueAdd, Price =ISNULL((Product.BasicPrice + ValueAdd),0), 
                                          Discounted = ISNULL((Select (Product.BasicPrice + (ValueAdd -(ValueAdd *(Discount*0.01))))
                                                 From DealerDiscount 
                                                 Where CategoryId = Product.GrandCategoryId AND DealerId='+@dealerid+'),0)
                                    From Product  Where 1=1 ';
                END
             ELSE
                BEGIN
                    SET @sql = '    Select Top(5) Id,ProdImage,ProductCode, ProductName,MrpPrice, BasicPrice,ValueAdd, Price =ISNULL((BasicPrice + ValueAdd),0), Discounted=0 
                                    From Product
                                    Where 1=1 ';
                END 

        END
    ELSE
        BEGIN
            IF @dealerid<>''
                BEGIN
                    SET @sql = '    Select  Product.Id,ProdImage,ProductCode,Collections.Name,Collections.Id, ProductName,MrpPrice, BasicPrice,ValueAdd, Price =ISNULL((Product.BasicPrice + ValueAdd),0), 
                                          Discounted = ISNULL((Select (Product.BasicPrice + (ValueAdd -(ValueAdd *(Discount*0.01))))
                                                 From DealerDiscount 
                                                 Where CategoryId = Product.GrandCategoryId AND DealerId='+@dealerid+'),0)
                                    FROM    Collections INNER JOIN
                                            Product ON Collections.Id = Product.CollectionId 
                                            where Product.CollectionId=@collectionId   AND 1=1 ';
                END
             ELSE
                BEGIN
                    SET @sql = '    Select  Product.Id,ProdImage,Collections.Name,Collections.Id,ProductCode, ProductName,MrpPrice, BasicPrice,ValueAdd, Price =ISNULL((BasicPrice + ValueAdd),0), Discounted=0 
                                    FROM    Collections INNER JOIN
                                            Product ON Collections.Id = Product.CollectionId 
                                            where Product.CollectionId=@collectionId AND 1=1 ';
                END 

            if (@GrandId > 0 and @ParentId>0 and @ChildId > 0 and @collectionId=0)
                Begin
                    Set @sql  = @sql + ' Product.ParentCategoryId = '+ Convert(Varchar, @ParentId)+' AND Product.GrandCategoryId = '+ Convert(Varchar, @GrandId)+' AND Product.ChitdCategoryId = '+Convert(varchar,@ChildId);
                End
            if (@GrandId > 0 and @ParentId>0 and @ChildId = 0 and @collectionId=0)
                Begin
                    Set @sql  = @sql + ' Product.ParentCategoryId = '+ Convert(Varchar, @ParentId)+' AND Product.GrandCategoryId = '+ Convert(Varchar, @GrandId)
                End
            if (@GrandId > 0 and @ParentId=0 and @ChildId = 0 and @collectionId=0)
                Begin
                    Set @sql  = @sql + ' AND Product.GrandCategoryId = '+ Convert(Varchar, @GrandId)
                End
        END

        exec(@sql)

END   

2 Answers 2

1

Rather than concatenating values into the TSQL, you should revise your current code to use sp_executesql and parameters in the generated TSQL, which avoids the risk of SQL injection in the generated TSQL, and allows query-plan re-use. For example:

SET @sql = '    Select Top(5) Product.Id,ProdImage,ProductCode, ProductName,MrpPrice, BasicPrice,ValueAdd, Price =ISNULL((Product.BasicPrice + ValueAdd),0), 
                                          Discounted = ISNULL((Select (Product.BasicPrice + (ValueAdd -(ValueAdd *(Discount*0.01))))
                                                 From DealerDiscount 
                                                 Where CategoryId = Product.GrandCategoryId AND DealerId=@dealerid),0)
                                    From Product  Where 1=1 ';
....
if (@GrandId > 0 and @ParentId>0 and @ChildId = 0 and @collectionId=0)
                Begin
                    Set @sql  = @sql + ' AND Product.ParentCategoryId = @ParentId AND Product.GrandCategoryId = @GrandId '
                End

In particular, note that I am not concatenating the current parameter values into the generated SQL.

You then call it as:

exec sp_executesql @sql, N'@dealerid int, @GrandId int',
                         @dealerid, @GrandId
                      -- ^^^ todo: add every parameter you need

The first parameter is the TSQL; the second parameter (N'@dealerid int, @GrandId int') is aliteral that describes the parameters via standard SQL syntax, then we map in the values to use. In this case, for convenience, we are using the same names - but that isn't a requirement.

Sign up to request clarification or add additional context in comments.

Comments

0

While building query, you need to append the variable as string rather than just assigning. Please try:

ALTER proc [dbo].[GetAllProductById] 
(
    @collectionId int,
    @GrandId int  ,
    @ParentId int ,
    @ChildId int,
    @dealerid varchar(50) 
)
As
Begin

Declare @sql as varchar(max)
 -- In case dealer is logged in ,then calculate the Discounted amount and return the same,
    -- else return Mrp and Our Price for all other customers

    IF  @collectionid<=0

        BEGIN
            IF @dealerid<>''
                BEGIN
                    SET @sql = '    Select Top(5) Product.Id,ProdImage,ProductCode, ProductName,MrpPrice, BasicPrice,ValueAdd, Price =ISNULL((Product.BasicPrice + ValueAdd),0), 
                                          Discounted = ISNULL((Select (Product.BasicPrice + (ValueAdd -(ValueAdd *(Discount*0.01))))
                                                 From DealerDiscount 
                                                 Where CategoryId = Product.GrandCategoryId AND DealerId='+@dealerid+'),0)
                                    From Product  Where 1=1 ';
                END
             ELSE
                BEGIN
                    SET @sql = '    Select Top(5) Id,ProdImage,ProductCode, ProductName,MrpPrice, BasicPrice,ValueAdd, Price =ISNULL((BasicPrice + ValueAdd),0), Discounted=0 
                                    From Product
                                    Where 1=1 ';
                END 

        END
    ELSE
        BEGIN
            IF @dealerid<>''
                BEGIN
                    SET @sql = '    Select  Product.Id,ProdImage,ProductCode,Collections.Name,Collections.Id, ProductName,MrpPrice, BasicPrice,ValueAdd, Price =ISNULL((Product.BasicPrice + ValueAdd),0), 
                                          Discounted = ISNULL((Select (Product.BasicPrice + (ValueAdd -(ValueAdd *(Discount*0.01))))
                                                 From DealerDiscount 
                                                 Where CategoryId = Product.GrandCategoryId AND DealerId='+@dealerid+'),0)
                                    FROM    Collections INNER JOIN
                                            Product ON Collections.Id = Product.CollectionId 
                                            where Product.CollectionId='+CAST(NVARCHAR(50), @collectionId)+'   AND 1=1 ';
                END
             ELSE
                BEGIN
                    SET @sql = '    Select  Product.Id,ProdImage,Collections.Name,Collections.Id,ProductCode, ProductName,MrpPrice, BasicPrice,ValueAdd, Price =ISNULL((BasicPrice + ValueAdd),0), Discounted=0 
                                    FROM    Collections INNER JOIN
                                            Product ON Collections.Id = Product.CollectionId 
                                            where Product.CollectionId='+CAST(NVARCHAR(50), @collectionId)+' AND 1=1 ';
                END 

            if (@GrandId > 0 and @ParentId>0 and @ChildId > 0 and @collectionId=0)
                Begin
                    Set @sql  = @sql + ' Product.ParentCategoryId = '+ Convert(Varchar, @ParentId)+' AND Product.GrandCategoryId = '+ Convert(Varchar, @GrandId)+' AND Product.ChitdCategoryId = '+Convert(varchar,@ChildId);
                End
            if (@GrandId > 0 and @ParentId>0 and @ChildId = 0 and @collectionId=0)
                Begin
                    Set @sql  = @sql + ' Product.ParentCategoryId = '+ Convert(Varchar, @ParentId)+' AND Product.GrandCategoryId = '+ Convert(Varchar, @GrandId)
                End
            if (@GrandId > 0 and @ParentId=0 and @ChildId = 0 and @collectionId=0)
                Begin
                    Set @sql  = @sql + ' AND Product.GrandCategoryId = '+ Convert(Varchar, @GrandId)
                End
        END

        exec(@sql)

END   

2 Comments

IMO, concatenating inputs is never a good way to do this, and can (at least, for [n][var]char) lead to SQL injection errors. sp_executesql would be a much more commendable approach.
Thanks @MarcGravell for your valuable tip. Stack overflow is a great place to study new things!

Your Answer

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