0

I am writing a procedure to produce an int output variable, but I'm not sure how to do this using dynamic sql. If I execute the below procedure I get the @AnlyNum value displayed in the results screen, but I just want @AnlyNum variable set with a value so I can use it. Thank you.

Create procedure [dbo].[sp_test] @Db varchar(50), @RwNum int, @AnlyNum int output

As

Begin

Declare @Sql nvarchar(max) =

'Select ''@AnlyNum'' = (Select AnlyId From '+@Db+'..Test order by AnlyId desc OFFSET '+convert(varchar(10),@RwNum)+' rows fetch next 1 rows only)'

End

exec(@Sql)
Thom A
  • 88,727
  • 11
  • 45
  • 75
CGarden
  • 169
  • 17
  • 3
    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 (and in this case, easily consume the `OUTPUT` parameter from the dynamic statement). – Thom A Mar 10 '22 at 13:49
  • 3
    Also, your code *is* **dangerous**. It is wide open to SQL injection attacks (as I allude to in the prior comment when using `EXEC (@SQL)` syntax). *Never* inject unsanitised values into a dynamic statement. Properly quote dynamic objects with `QUOTENAME` and properly parametrise parameters. – Thom A Mar 10 '22 at 13:51
  • 3
    Also, 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 10 '22 at 13:52
  • Thank you for that much appreciated. – CGarden Mar 10 '22 at 16:25

1 Answers1

2

This removes SQL injection concerns by properly escaping the database name and also dynamically executing against that database instead of embedding the database name in the command. Also, you don't need @RwNum to be dynamic.

CREATE PROCEDURE dbo.test
  @Db      sysname,
  @RwNum   int,
  @AnlyNum int output
AS
BEGIN
  SET NOCOUNT ON;

  DECLARE @exec nvarchar(max) = QUOTENAME(@Db) + N'.sys.sp_executesql',
          @sql nvarchar(max) = N'SELECT @AnlyNum = AnlyId 
            From dbo.Test order by AnlyId desc 
            OFFSET @RwNum rows fetch next 1 rows only);';

  EXEC @exec @sql, N'@AnlyNum int output, @RwNum int',
    @AnlyNum output, @RwNum;
END
Aaron Bertrand
  • 272,866
  • 37
  • 466
  • 490
  • Thank you. Why don't I need @RwNum to be dynamic? – CGarden Mar 10 '22 at 16:28
  • Could you also explain a bit more about what the code is doing, as this is not functionality I have used before. Thank you. – CGarden Mar 10 '22 at 16:43
  • @CGarden You don't need `@RwNum` to be dynamic because [it accepts constants _or expressions_](https://learn.microsoft.com/en-us/sql/t-sql/queries/select-order-by-clause-transact-sql?view=sql-server-ver15#syntax). This means you can just say `DECLARE @r int; SELECT ... ORDER BY ... OFFSET @r ROWS ...` - you do need dynamic SQL because you have made the database name dynamic. For the rest see [this answer](https://dba.stackexchange.com/a/185047/1186). – Aaron Bertrand Mar 10 '22 at 16:53