passing parameters into SQL server - "must declare scalar variable"

Question

EDIT:

In an effort to clean up the SQL just a little, I've written the code as PeterHE suggested, and hardcoded in (just for testing) the test values. Now I'm getting a new error message: "in_eq_Equipment.in_eq_CategoryID_fk" could not be bound."

Here's the revised test code:

    select in_eq_ID, in_eq_TagNumber as TagNumber, Title1Item, in_eq_AssetDescription as Description, in_eq_ExtendedDescription as ExtendedDescription, in_eq_SerialNumber as SerialNumber, in_eq_ValuationAmount as TotalValue, in_eq_CustodianName as Name, in_eq_ComplexBuilding as ShortLocation, in_eq_SubLocationCode as ShortRoomNumber, in_ca_Categories.in_ca_CategoryName as CategoryName, in_eq_DispositionDate as DispositionDate, Departments.DepartmentCode, DATEADD (dd, 0, DATEDIFF (dd, 0, in_eq_Equipment.in_eq_AcquisitionDate)) as AcquisitionDate

from in_eq_Equipment, Departments

LEFT JOIN in_ca_Categories ON in_eq_Equipment.in_eq_CategoryID_fk = in_ca_Categories.in_ca_CategoryID       
where in_eq_Equipment.DepartmentID = CAST ('00000000-0000-0000-0000-000000000000' AS nvarchar(36)) and upper (in_eq_AssetDescription) LIKE upper ('%T$')

Rusty old programmer here returning... trying to test a stored procedure and running into this error message. Error:

Msg 137, Level 15, State 2, Line 18 Must declare the scalar variable "@DepartmentID".

// The test calling code:

DECLARE @SearchString varchar(30), @DispositionText varchar(200)
declare @DepartmentID uniqueidentifier;

SET @SearchString = 'T'
SET @DispositionText = '';
SET @DepartmentID = '00000000-0000-0000-0000-000000000000';
EXEC GetInventoryByAssetDescription @SearchString, @DispositionText, @DepartmentID

Actual stored procedure code (Yes, I know it's bad code--I didn't write it originally, but must modify it within a short timeframe):

USE [Inventory]
GO
/****** Object:  StoredProcedure [dbo].[GetInventoryByAssetDescription]    Script Date: 12/23/2019 9:09:51 AM ******/
SET ANSI_NULLS OFF
GO
SET QUOTED_IDENTIFIER OFF
GO
ALTER PROCEDURE [dbo].[GetInventoryByAssetDescription]
      (
            @SearchString varchar(30),
            @DispositionText varchar(200) = null,
            @DepartmentID uniqueidentifier
      )  
AS
begin
    SET NOCOUNT ON
    declare @sql nvarchar (2000)

    select @SearchString=UPPER(@SearchString)
    set @sql = '   select in_eq_ID,
        in_eq_TagNumber as TagNumber,
        Title1Item,
        in_eq_AssetDescription as Description,
        in_eq_ExtendedDescription as ExtendedDescription,
        in_eq_SerialNumber as SerialNumber,
        in_eq_ValuationAmount as TotalValue,
        in_eq_CustodianName as Name,
        in_eq_ComplexBuilding as ShortLocation,
        in_eq_SubLocationCode as ShortRoomNumber,
        in_ca_Categories.in_ca_CategoryName as CategoryName,
        in_eq_DispositionDate as DispositionDate,
        Departments.DepartmentCode,
        DATEADD (dd, 0, DATEDIFF (dd, 0, in_eq_Equipment.in_eq_AcquisitionDate)) as AcquisitionDate
        from in_eq_Equipment, Departments
        where in_eq_Equipment.DepartmentID = @DepartmentID
            LEFT JOIN in_ca_Categories ON in_eq_Equipment.in_eq_CategoryID_fk = in_ca_Categories.in_ca_CategoryID
            WHERE upper (in_eq_AssetDescription) LIKE upper ('''+ @SearchString + ''')
            '


        set  @sql=@sql+'   ' + ISNULL(@DispositionText,' ')  + '  order by in_eq_AssetDescription'
        execute (@sql)
        return
end

Yes it is bad. Not just bad - terrible. There are lots of problems with it. Duplicate upper logic, old style joins, what appears to be a faulty join between equipment and departments, dispositiontext can easily cause an error. And most importantly - susceptible to SQL INJECTION. — SMor
– SMor, Commented Dec 23, 2019 at 15:45
If you are allowed to modify the stored proc, why are you using dynamic SQL because it performs poor. There is nothing in your SQL SELECT that needs to dynamic parameters. All are straight forward, taking just from inputs. I would say, just remove that dynamic sql logic, have a normal SQL SELECT statement which just queries your respective tables based on input parameters. — sam
– sam, Commented Dec 23, 2019 at 15:51
This original SQL was apparently created in 2005. I am so rusty and under a timeline, doubt I have time to clean it up completely. Just trying to make a minor modification at this point. Thanks for your input. — John
– John, Commented Dec 23, 2019 at 15:55
What you have doesn't make sense. What is @DispositionText? You just inject it into your statement, which makes no sense. Why are you cross joining to departments? Shouldn't it be an INNER JOIN and then @DepartmentID is compared against the column in the departments table? Why do you have 2 WHERE clauses as well? — Thom A
– Thom A ♦, Commented Dec 23, 2019 at 15:56
"This original SQL was apparently created in 2005" and would never have worked in 2005. FROM TAble1, Table2 WHERE Table1.Col = 1 LEFT JOIN Table3 ON... is complete wrong syntax. — Thom A
– Thom A ♦, Commented Dec 23, 2019 at 15:57

PeterHe · Accepted Answer · 2019-12-23 15:40:47Z

1

The where clause:

where in_eq_Equipment.DepartmentID = @DepartmentID

Should be:

where in_eq_Equipment.DepartmentID = '''+CAST(@DepartmentID AS nvarchar(36))+'''

P.S.: It does not look like you need dynamic sql at all. Also even if you need, should change to use sp_executesql with parameters.

answered Dec 23, 2019 at 15:40

PeterHe

2,7861 gold badge11 silver badges7 bronze badges

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

8 Comments

Thom A Over a year ago

No, it shouldn't, it should be a parameter. This leaves the SP wide open to injection. This answer is dangerous because it doesn't address the problem, and actually further endorses injection..

SMor Over a year ago

But Peter's suggestion does correct OP's error. It is not the best solution but it works.

Thom A Over a year ago

It does "work", yes @SMor , just like a WHILE works for inserting many rows from one table to another, or using adhesive tap to fix a leaking pipe. Doesn't mean you should do it though.

Thom A Over a year ago

Yes, but you should actually show that, @PeterHe . Especially as the SQL isn't dynamic at all; there is literally no need for the injection, and therefore huge security vulnerability.

PeterHe Over a year ago

@Larnu, NO. He needs to try that. The answer gives the solution.

|

sam · Accepted Answer · 2019-12-23 19:45:59Z

1

Dynamic SQLs are very bad. I am just giving the code which prevents the errors that you are seeing:

USE [Inventory]
GO
/****** Object:  StoredProcedure [dbo].[GetInventoryByAssetDescription]    Script Date: 12/23/2019 9:09:51 AM ******/
SET ANSI_NULLS OFF
GO
SET QUOTED_IDENTIFIER OFF
GO
ALTER PROCEDURE [dbo].[GetInventoryByAssetDescription]
      (
            @SearchString varchar(30),
            @DispositionText varchar(200) = null,
            @DepartmentID uniqueidentifier
      )  
AS
begin
    SET NOCOUNT ON
    declare @sql nvarchar (2000)

select @SearchString=UPPER(@SearchString)
set @sql = '   select in_eq_ID,
    in_eq_TagNumber as TagNumber,
    Title1Item,
    in_eq_AssetDescription as Description,
    in_eq_ExtendedDescription as ExtendedDescription,
    in_eq_SerialNumber as SerialNumber,
    in_eq_ValuationAmount as TotalValue,
    in_eq_CustodianName as Name,
    in_eq_ComplexBuilding as ShortLocation,
    in_eq_SubLocationCode as ShortRoomNumber,
    in_ca_Categories.in_ca_CategoryName as CategoryName,
    in_eq_DispositionDate as DispositionDate,
    Departments.DepartmentCode,
    DATEADD (dd, 0, DATEDIFF (dd, 0, in_eq_Equipment.in_eq_AcquisitionDate)) as AcquisitionDate
    from in_eq_Equipment, Departments
        LEFT JOIN in_ca_Categories ON in_eq_Equipment.in_eq_CategoryID_fk = in_ca_Categories.in_ca_CategoryID
        WHERE 
           in_eq_Equipment.DepartmentID = '''+CAST(@DepartmentID AS nvarchar(36))+'''
           upper (in_eq_AssetDescription) LIKE upper ('''+ @SearchString + ''')' --searchString is already converted UPPER CASE at the beginning, not sure why its converted here again.

    set  @sql=@sql+'   ' + ISNULL(@DispositionText,' ')  + '  order by in_eq_AssetDescription'
    execute (@sql)
    return

end

And below is the refactored stored procedure which directly takes your SQL inputs and returns the resultset:

USE [Inventory]
GO
SET ANSI_NULLS OFF
GO
SET QUOTED_IDENTIFIER OFF
GO
ALTER PROCEDURE [dbo].[GetInventoryByAssetDescription]
      (
            @SearchString varchar(30),
            @DispositionText varchar(200) = null,
            @DepartmentID uniqueidentifier
      )  
AS
begin
    SET NOCOUNT ON

    SET @SearchString = '''%' + UPPER('test') + ''''
select in_eq_ID
       , in_eq_TagNumber as TagNumber
       , Title1Item
       , in_eq_AssetDescription as [Description]
       , in_eq_ExtendedDescription as ExtendedDescription
       , in_eq_SerialNumber as SerialNumber
       , in_eq_ValuationAmount as TotalValue
       , in_eq_CustodianName as [Name]
       , in_eq_ComplexBuilding as ShortLocation
       , in_eq_SubLocationCode as ShortRoomNumber
       , in_ca_Categories.in_ca_CategoryName as CategoryName
       , in_eq_DispositionDate as DispositionDate
       , Departments.DepartmentCode
       , DATEADD (dd, 0, DATEDIFF (dd, 0, in_eq_Equipment.in_eq_AcquisitionDate)) as AcquisitionDate
from      in_eq_Equipment
JOIN      Departments      ON in_eq_equipment.departmentid = departments.id
LEFT JOIN in_ca_Categories ON in_eq_Equipment.in_eq_CategoryID_fk = in_ca_Categories.in_ca_CategoryID       
where (in_eq_Equipment.DepartmentID =  @DepartmentID OR @DepartmentID = '00000000-0000-0000-0000-000000000000') 
     and upper (in_eq_AssetDescription) LIKE @SearchString 
     --AND ISNULL(@DispositionText,' ') = ' '  -- Not able to understand this condition from your dynamic SQL
order by in_eq_AssetDescription  
        return
end

edited Dec 23, 2019 at 19:45

answered Dec 23, 2019 at 16:29

sam

1,9951 gold badge11 silver badges16 bronze badges

10 Comments

John Over a year ago

I'm actually in the middle of refactoring the code, but your second code slice is getting the same error I am: Msg 4104, Level 16, State 1, Procedure GetInventoryByAssetDescription, Line 33 The multi-part identifier "in_eq_Equipment.in_eq_CategoryID_fk" could not be bound.

sam Over a year ago

@Brian My second code slice just replaces the same query from your POST with direct SQL instead of dynamic SQL. That error means, in_eq_CategoryID_fk column is not found in in_eq_Equipment table. Without knowing your tables structures, its hard to fix that error.

John Over a year ago

Thanks, Sam... but in_eq_CategoryID_fk actually IS in the in_eq_Equipment table. That's what causing my confusion.

sam Over a year ago

How are in_eq_Equipment, Departments joined?

John Over a year ago

in_eq_equipment.departmentid = departments.id I am rewriting the SP now... going to take a bit because I am so rusty. :(

|

Collectives™ on Stack Overflow

passing parameters into SQL server - "must declare scalar variable"

2 Answers 2

8 Comments

10 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

8 Comments

10 Comments

Your Answer

Sign up or log in

Post as a guest

Related