0
votes

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
2
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
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
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
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? - Larnu
"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. - Larnu

2 Answers

1
votes

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.

1
votes

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