-4

Help me with this Dynamic statement perform faster, the statement will fetch top n values for each column from a table.

The table will have an "n" number of columns but will have a primary key. NULLs couldn't have been avoided as any other value is considered as VALID and should go to the database.

Table

+-------+------+------+------+
| Depth | RPMA | ROP  | WOB  |
+-------+------+------+------+
|  6111 |   72 | 14.6 | 0    |
|  6110 |   72 | 14.1 | 1    |
|  6109 |   66 | 15.2 | NULL |
|  6108 |   68 | 14   | NULL |
|  6107 |   69 | 14   | NULL |
|  6106 |   61 | 14.8 | NULL |
|  6105 |   70 | NULL | NULL |
|  6104 |   64 | NULL | NULL |
|  6103 |   59 | NULL | NULL |
|  6102 |   49 | NULL | NULL |
+-------+------+------+------+

Result set,

+-------+------+------+------+
| Depth | RPMA | ROP  | WOB  |
+-------+------+------+------+
|  6111 | 72   | NULL | 0    |
|  6110 | 72   | NULL | 1    |
|  6109 | NULL | 15.2 | NULL |
|  6106 | NULL | 14.8 | NULL |
+-------+------+------+------+

Dynamic SQL used to get current result set,

DECLARE @Columns VARCHAR(MAX); -- Param1
DECLARE @IdxColumn VARCHAR(250); --Param2
DECLARE @Limit VARCHAR(11); --Param3
DECLARE @SQL NVARCHAR(MAX)=''; --Param4

DECLARE @query NVARCHAR(MAX) = ' SELECT TOP (' + @pLimit + ') ' + @IdxColumn + ', ' + @Columns + ' FROM [Table] WHERE '

SET @SQL = @query + REPLACE(@Columns,',', ' IS NOT NULL ORDER BY '+ @IdxColumn + ' ASC ' + N' UNION' + @query) + ' IS NOT NULL ORDER BY ' + @IdxColumn

SET @SQL = 'SELECT * FROM ('+@SQL+') T ORDER BY ' + @IdxColumn + ' ASC'   

EXEC (@SQL)
Dani Mathew
  • 808
  • 10
  • 18
  • @DaniMathew For column Depth, what will be the top N ? How to get the value of N ? – Amira Bedhiafi Nov 26 '19 at 13:26
  • N will be passed into the procedure as a parameter, it will be used along with Top, and the statement is constructed as varchar then gets executed using sp_executesql – Dani Mathew Nov 26 '19 at 13:29

2 Answers2

2

The following query should work for the sample data:

WITH cte AS (
  SELECT *
       , DENSE_RANK() OVER (ORDER BY RPMA DESC) AS RPMA_RANK
       , DENSE_RANK() OVER (ORDER BY ROP DESC) AS ROP_RANK
       , DENSE_RANK() OVER (ORDER BY WOB DESC) AS WOB_RANK
  FROM t
)
SELECT Depth
     , CASE WHEN RPMA_RANK <= 2 THEN RPMA END
     , CASE WHEN ROP_RANK <= 2 THEN ROP END
     , CASE WHEN WOB_RANK <= 2 THEN WOB END
FROM cte
WHERE RPMA_RANK <= 2
OR ROP_RANK <= 2
OR WOB_RANK <= 2

Note that it returns three rows for RPMA column (there are two 72 and one 70). For n number of columns you need to use dynamic SQL.

Salman A
  • 262,204
  • 82
  • 430
  • 521
2

This doesn't answer the question, but does fix the terrifying security flaws in the above.

There are multiple problems with the above, so please note that this is a significant but needed change to the SQL you have. Right now you are injecting unsantised parameters into your code, and also using datatypes that are vastly too large. @Columns is varchar(MAX), meaning that someone has 2GB worth of characters to inject into your system. @IdxColumn is a varchar(250) and references a single column; an object can at most be 128 characters long so there's no need for the other 122 characters. Also @Limit is a varchar, despite being an int and should be a parameter.

Firstly, rather than using a varchar(MAX) for @Columns I suggest a table type object:

CREATE TYPE dbo.ObjectList (ObjectName sysname);

sysname is a synonym of nvarchar(128) NOT NULL; and is the data type used for object names in SQL Server. You'll then need to INSERT the names of the columns into a declared table type parameter; one row for each Column Name

Then we can safely inject and parametrise your query:

--Parameters
DECLARE @Columns dbo.ObjectList,
        @IdxColumn sysname, --sysname as well
        @Limit int; --not varchar

--Variables needed in the SQL:

DECLARE @SQL nvarchar(MAX),
        @CRLF nchar(2) = NCHAR(13) + NCHAR(10);

SET @SQL = N'SELECT TOP (@Limit)' + @CRLF + 
           N'           ' + QUOTENAME(@IdxColumn) + N',' + @CRLF +
           STUFF((SELECT N',' + @CRLF +
                         N'           ' + QUOTENAME(C.ObjectName)
                  FROM @Columns C
                  FOR XML PATH(N''),TYPE).value('.','nvarchar(MAX)'),1,3,N'') + @CRLF +
           N'FROM dbo.[Table]' + @CRLF + --Should dbo.[Table] also not be safely injected?
           N'WHERE ' +
           STUFF((SELECT @CRLF + 
                         N'   OR ' + QUOTENAME(C.ObjectName) + N' IS NOT NULL'
                  FROM @Columns C
                  FOR XML PATH(N''),TYPE).value('.','nvarchar(MAX)'),1,8,N'') + @CRLF + 
          N'ORDER BY ' + QUOTENAME(@IdxColumn) + N' ASC;'

EXEC sp_executesql @SQL, N'@Limit int', @Limit;
Thom A
  • 88,727
  • 11
  • 45
  • 75