1

I will pass a table-valued input parameter into a stored procedure, and also a variable that contains query string, so I made my sproc like this.

CREATE PROCEDURE [dbo].[SP_SelectData_View]
(
    @Sort VARCHAR(MAX),
    @CONDITION VARCHAR(MAX) = ''
    @Values dbo.FlowStatus READONLY
)
AS
BEGIN
    DECLARE @STRQUERY NVARCHAR(MAX)

    IF @CONDITION IS NOT NULL AND @CONDITION != ''
    BEGIN
        SET @CONDITION = 'WHERE ' + @CONDITION
    END
    ELSE
    BEGIN
        SET @CONDITION = ''
    END

    IF @Sort IS NULL OR @Sort = ''
    BEGIN
        SET @Sort = 'Id Desc'
    END

    BEGIN
        SET @STRQUERY = 'SELECT A.* 
        FROM ' + @Values + ' as FlowStatus' 
        JOIN Tbl_A as A
        ON A.status = FlowStatus.StatusNowId AND B.flow = FlowStatus.FlowNowId 
        ' + @CONDITION + '
        Order By ' + @Sort

        EXEC(@STRQUERY)
    END
END

But in the code above, I got an error

must declare scalar variable @Values

I've searched for it and I think it is because the aliases is not detected because it's inside a string. But if I didn't put it in a string query, the @condition and @sort variable will be error. Is there a solution where I can do both calling the table-valued variable and query string variable together?

Dale K
  • 25,246
  • 15
  • 42
  • 71
stucknubie
  • 33
  • 6
  • 2
    You will get that error when you are not passing a value for `@Values`. You need to show how you are calling it. You also have a syntax error, a missing comma between your second and third parameter. – Dale K Mar 15 '22 at 09:51
  • 6
    There are so many problems with the above. The start with, as an FYI the prefix `sp_` is reserved, by Microsoft, for **S**pecial / **S**ystem **P**rocedures. It should *not* be used for User Procedures. Doing so comes with a performance cost and the risk of your Procedure simply not working one day after an update/upgrade. Either use a different prefix or (possibly better) no prefix at all. [Is the sp_ prefix still a no-no?](https://sqlperformance.com/2012/10/t-sql-queries/sp_prefix). – Thom A Mar 15 '22 at 09:52
  • 5
    Next, it's generally advised to not use syntax such as `EXEC (@SQL);`. Such statements cannot be parametrised, which promote bad habits that result in security flaws like SQL injection. If you need to run a statement that is within a variable or literal string then use [`sys.sp_executesql`](https://learn.microsoft.com/en-us/sql/relational-databases/system-stored-procedures/sp-executesql-transact-sql). Then you can easily parametrise the statement if you need to. (This is why liekly why you're getting the error you are). – Thom A Mar 15 '22 at 09:53
  • 5
    Finally, passing an entire `WHERE` and/or `ORDER BY` clause is a sure fire way to open yourself up to SQL injection attacks. The code above is *extremely* dangerous. What ever the goal you're trying to achieve it, I am completely certain it is an [XY Problem](//xyproblem.info). This type of "solution" is never correct, and especially isn't in the SQL layer. If you "must" be building queries (from you application?) there are **significantly** safer ways to do this. I suggest you explain what the *real* goal you're trying to achieve here is. – Thom A Mar 15 '22 at 09:55
  • 1
    Actually I think table variables go out of scope when you execute SQL. – Dale K Mar 15 '22 at 09:56
  • 1
    Also, you appear to try to use a table variable like a scalar variable; that won't work You can't concatenate a `table` to a `varchar`; that doesn't make any sense. – Thom A Mar 15 '22 at 10:08
  • This may be useful to read, I learnt a lot from Brent Ozar https://www.brentozar.com/sql/dynamic/ but yea seems strange that you are passing some SQL, I would consider passing the parameters to this stored proc and then build the SQL as brent ozar shows. I believe he also shows how to protect against sql injection. Also how to call the dynamic SQL passing the parameters into the actual EXEC sp_executesql. I also presume your Flows Status is a user defined table type https://www.sqlshack.com/table-valued-parameters-in-sql-server/ – Andrew Mar 15 '22 at 11:18

1 Answers1

1

There are several things wrong with the approach you currently have, as I and others have commented, Brent Ozar has a good reference on dynamic SQL https://www.brentozar.com/sql/dynamic/

I would say don't pass in some SQL, construct it in the stored proc; passing in parameters such as name which is used in the where, hence I have put a full working example. This also shows how to pass the user defined table type into the stored proc and then also pass it into the dynamic SQL.

I hope this is a good enough example of the techniques, I had a bit of time so thought I would try and help as much as possible :)

/*
--------------------------------------------
Create a test table to run the stored proc against
*/
IF (NOT EXISTS (SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA = 'dbo' AND TABLE_NAME = 'MyTestTable'))
BEGIN
    PRINT 'Creating table MyTestTable'

    CREATE TABLE [dbo].[MyTestTable](
        Id BIGINT NOT NULL IDENTITY(1,1) PRIMARY KEY,
        [Name] NVARCHAR(50) NOT NULL
    )

    INSERT INTO dbo.MyTestTable ([Name])
    VALUES ('Andrew'), 
    ('Bob'), 
    ('john')
    
    -- SELECT * FROM MyTestTable

END
GO

/*
--------------------------------------------
Create the table type that we pass into the store proc
*/

IF NOT EXISTS (SELECT * FROM sys.types WHERE is_table_type = 1 AND name = 'FlowStatus')
BEGIN
    PRINT 'Creating type [dbo].[FlowStatus]'
    CREATE TYPE [dbo].FlowStatus AS TABLE (
        MyId BIGINT PRIMARY KEY, 
        SomeText NVARCHAR(200)
    )
END 
GO

/*
--------------------------------------------
Create the stored proc with the User Defined table type
*/
CREATE OR ALTER PROCEDURE [dbo].[MyStoredProc]
(
    @SortBy VARCHAR(50),
    @SearchName VARCHAR(50),
    @Values dbo.FlowStatus READONLY
)
AS
BEGIN
    -- As your SQL gets more complex it is an idea to create seperate parts of the SQL
    DECLARE @SqlToExecute NVARCHAR(MAX)

    -- The primary data you want to get
    SET @SqlToExecute = N'
        SELECT T.Id, T.[Name], V.SomeText 
        FROM MyTestTable AS T
        LEFT JOIN @Values AS V ON V.MyId = T.Id 
        WHERE 1 = 1' -- We do this so that we can have many AND statements which could be expanded upon

    IF @SearchName IS NOT NULL
    BEGIN
        SET @SqlToExecute = @SqlToExecute + N'
        AND T.[Name] LIKE ''%' + @SearchName + ''''
    END

    IF @SortBy IS NOT NULL
    BEGIN
        SET @SqlToExecute = @SqlToExecute + N'
        ORDER BY ' + 
        CASE WHEN @SortBy LIKE 'Name%' THEN N'T.[Name]'
        ELSE N'T.[Id]'
        END
    END

    -- Print out the script that will be run, useful for debugging you code
    PRINT @SqlToExecute

    EXEC sp_executesql @SqlToExecute,
        N'@Values dbo.FlowStatus READONLY', @Values
    
END
GO

/*
--------------------------------------------
Now lets test it
-- Test Andrew
*/
DECLARE @flowStatusType AS dbo.FlowStatus

INSERT INTO @flowStatusType(MyId, SomeText)
VALUES(1, 'Test1'),
(2, 'Test2')

EXEC [dbo].[MyStoredProc] @SearchName = 'Andrew', @SortBy = 'Name', @Values = @flowStatusType
GO

-- Test Bob
DECLARE @flowStatusType AS dbo.FlowStatus

INSERT INTO @flowStatusType(MyId, SomeText)
VALUES(1, 'Test1'),
(2, 'Test2')

EXEC [dbo].[MyStoredProc] @SearchName = 'Bob', @SortBy = 'Name', @Values = @flowStatusType
GO

Its also worth noting that if you can just join on the @Values without needing dynamic SQL then that is sure to be less work.

Andrew
  • 2,571
  • 2
  • 31
  • 56