# Tsql Review

> Analyze raw T-SQL source code for anti-patterns, security risks, and static performance smells. Applies 85 checks (T1–T85) across structural, correctness, security, deprecated syntax, performance, and SQL 2017–2022 modern syntax categories. Use this skill whenever a user pastes a stored procedure, function, view, trigger, or ad-hoc SQL and asks for a review; asks if code is safe, correct, or opti…

- **Type:** Skill
- **Install:** `agentstack add skill-vanterx-mssql-performance-skills-tsql-review`
- **Verified:** Pending review
- **Seller:** [vanterx](https://agentstack.voostack.com/s/vanterx)
- **Installs:** 0
- **Category:** [Databases](https://agentstack.voostack.com/c/databases)
- **Latest version:** 0.1.0
- **License:** MIT
- **Upstream author:** [vanterx](https://github.com/vanterx)
- **Source:** https://github.com/vanterx/mssql-performance-skills/tree/main/skills/tsql-review

## Install

```sh
agentstack add skill-vanterx-mssql-performance-skills-tsql-review
```

Requires the [AgentStack CLI](https://agentstack.voostack.com/docs/cli). Works with Claude Code, Cursor, and any MCP-compatible agent.

## About

# T-SQL Static Review Skill

## Purpose

Analyze T-SQL source code — stored procedures, ad-hoc queries, scripts, migration files — for anti-patterns that are detectable without running the query or capturing an execution plan. Covers 85 checks (T1–T85) across six categories: structural anti-patterns, correctness and logic, security and dynamic SQL, deprecated and non-idiomatic syntax, performance smells, and SQL Server 2017–2022 modern syntax checks.

This is the "shift-left" complement to `sqlplan-review`. Run it during code review to catch problems before they reach production. Run `sqlplan-review` on the resulting execution plan to catch what only surfaces at runtime.

## Input

Accept any of:
- Raw T-SQL source code (paste inline or provide a file path)
- A `.sql` file path
- A description of the query structure ("a stored proc with a cursor that builds a dynamic WHERE clause")

If the user provides a file path, read the file and analyze its content. If the input is inline SQL, analyze it directly. If the input is a description, apply the checks based on what is described and note which checks could not be verified from the description alone.

## How to Run

Walk T1–T85 in category order. Report every triggered finding — do not stop at the first match. For checks where the SQL construct is absent, note them as passing in the Passed Checks section. For checks where schema or parameter type information is unknown, state your assumption explicitly rather than skipping the check.

---

## Thresholds Reference

| Metric | Value |
|--------|-------|
| CTE chain depth warning | > 4 levels deep |
| Large IN list | > 20 discrete values in an IN() clause |
| Nested subquery depth | ≥ 3 levels of nested scalar subqueries |
| Excessive parameters | > 50 named parameters in a stored procedure |
| Wide index suggestion | > 4 key columns OR > 5 INCLUDE columns |
| NOLOCK overuse threshold | ≥ 3 tables WITH (NOLOCK) in the same query |
| Small variable-length type | ≤ 2 characters (VARCHAR(1), VARCHAR(2), NVARCHAR(1), NVARCHAR(2)) |

---

## Structural Anti-Patterns (T1–T15, T51–T55)

Run these checks for patterns that prevent index usage, expand data volumes unnecessarily, or indicate set-based logic replaced by row-by-row processing.
### T1 — SELECT * (No Explicit Column List)
- **Trigger:** `SELECT *` in any SELECT statement (including SELECT INTO, subqueries, CTEs, or views)
- **Severity:** Warning
- **Fix:** Replace `*` with an explicit column list. Eliminates surprise column additions when the schema changes, prevents over-fetching wide rows, and allows the optimizer to consider covering indexes.
### T2 — Missing WHERE on UPDATE or DELETE
- **Trigger:** An `UPDATE` or `DELETE` statement with no `WHERE` clause (including `TRUNCATE`-equivalent patterns using DELETE)
- **Severity:** Critical
- **Fix:** Add a `WHERE` clause or, if a full-table wipe is intended, use `TRUNCATE TABLE` (which is faster and fully logged). If the omission is intentional, add a comment explaining the intent.
### T3 — Missing WHERE on SELECT (Full-Table Read)
- **Trigger:** A `SELECT` or `SELECT INTO` with no `WHERE` clause on a named user table (not a system view or TVF with no filter parameter)
- **Severity:** Info
- **Fix:** Confirm the full-table read is intentional. Add `WHERE 1=1 -- intentional full scan` as documentation if it is. Otherwise add a predicate.
### T4 — Non-Sargable Predicate — Function Wrapping or Arithmetic on Indexed Column
- **Trigger:** A function call or arithmetic expression in a `WHERE`, `HAVING`, or `JOIN ON` clause that wraps or involves a column reference: `YEAR(col)`, `MONTH(col)`, `DAY(col)`, `CAST(col AS ...)`, `CONVERT(type, col)`, `UPPER(col)`, `LOWER(col)`, `LEFT(col, n)`, `SUBSTRING(col, 1, n)`, `ISNULL(col, default)`, `COALESCE(col, ...)`, or arithmetic on the column side: `col + n`, `col - n`, `col * n`, `col / n`. For DATEDIFF specifically see T60; for LEN/DATALENGTH see T74.
- **Severity:** Warning
- **Fix:** Rewrite the predicate so the column is bare and the transformation is applied to the literal or parameter. Example: `WHERE YEAR(OrderDate) = 2024` → `WHERE OrderDate >= '2024-01-01' AND OrderDate  NULL` or `col != NULL` (instead of `IS NULL` / `IS NOT NULL`)
- **Severity:** Critical
- **Fix:** `= NULL` always evaluates to UNKNOWN in SQL Server (regardless of SET ANSI_NULLS setting in modern compatibility levels). Use `IS NULL` or `IS NOT NULL`. If comparing two nullable columns, use `col1 IS NOT DISTINCT FROM col2` (SQL Server 2022+) or `(col1 = col2 OR (col1 IS NULL AND col2 IS NULL))`.
### T17 — Outer Join Nullified by WHERE Filter on Right-Side Column
- **Trigger:** A `LEFT JOIN` or `RIGHT JOIN` where the `WHERE` clause filters on a non-NULLable column from the outer (optional) side of the join: `LEFT JOIN T2 ON ... WHERE T2.col = @val`
- **Severity:** Warning
- **Fix:** A WHERE filter on the right-side column of a LEFT JOIN eliminates the NULL rows produced by the outer join, effectively converting it to an INNER JOIN — often unintentionally. Move the filter into the JOIN ON condition if outer rows should be preserved: `LEFT JOIN T2 ON T2.id = T1.id AND T2.col = @val`. Use INNER JOIN explicitly if you truly mean to eliminate non-matching rows.
### T18 — Missing ORDER BY in Final SELECT
- **Trigger:** A `SELECT` statement intended for ordered display (returned to a caller, top-level statement, or `SELECT INTO`) with no `ORDER BY`
- **Severity:** Info
- **Fix:** Without `ORDER BY`, SQL Server may return rows in any order — including different orders on different executions depending on available parallelism and I/O patterns. Add an explicit `ORDER BY` on a deterministic key. Exception: queries used as subqueries or CTEs where order is irrelevant.
### T19 — Missing TRY/CATCH Around DML
- **Trigger:** An `INSERT`, `UPDATE`, `DELETE`, or `MERGE` statement in a stored procedure, trigger, or multi-statement batch with no enclosing `BEGIN TRY / BEGIN CATCH` block
- **Severity:** Warning
- **Fix:** Wrap DML in `BEGIN TRY ... END TRY BEGIN CATCH ... END CATCH`. Use `THROW` (SQL Server 2012+) in the CATCH block to re-raise the error. Log the error using `ERROR_NUMBER()`, `ERROR_MESSAGE()`, `ERROR_LINE()` before re-throwing. Without error handling, a failed DML may leave partial state.
### T20 — Multi-Statement DML Without Explicit Transaction
- **Trigger:** Two or more `INSERT`, `UPDATE`, `DELETE`, or `MERGE` statements in the same batch or procedure with no `BEGIN TRANSACTION / COMMIT / ROLLBACK` wrapping them
- **Severity:** Info
- **Fix:** If the statements must succeed or fail atomically, wrap in `BEGIN TRANSACTION ... COMMIT`. Include error handling with `ROLLBACK` in the CATCH block. If the statements are intentionally independent, document it with a comment.
### T21 — UNION Instead of UNION ALL
- **Trigger:** `UNION` keyword (not `UNION ALL`) combining result sets
- **Severity:** Info
- **Fix:** `UNION` sorts both result sets and eliminates duplicates — equivalent to `UNION ALL` plus `SELECT DISTINCT`. This is expensive and usually unnecessary. Use `UNION ALL` unless duplicate elimination is genuinely required. If duplicates are expected and unwanted, investigate the root cause rather than relying on UNION to hide them.
### T22 — CASE Branches With Mismatched Return Types
- **Trigger:** A `CASE` expression whose WHEN branches return values of different data types that require implicit conversion to unify (e.g., one branch returns `INT`, another returns `VARCHAR`)
- **Severity:** Warning
- **Fix:** SQL Server resolves CASE branch type mismatches by promoting to the highest-precedence type. This can cause implicit conversions or data truncation. Use explicit `CAST` or `CONVERT` in each branch to the desired final type.
### T23 — Missing ELSE in CASE Expression
- **Trigger:** A `CASE` expression with no `ELSE` clause
- **Severity:** Info
- **Fix:** Without ELSE, a CASE returns NULL when no WHEN matches. If NULL is the intended behavior for unmatched rows, document it with an explicit `ELSE NULL`. If a default value is needed, add `ELSE default_value`. This makes the behavior explicit and prevents accidental NULLs.
### T24 — CTE Referenced More Than Once
- **Trigger:** A Common Table Expression (CTE) whose name appears in more than one FROM clause or subquery within the same statement
- **Severity:** Info
- **Fix:** SQL Server does not materialize CTEs — each reference to the CTE re-executes its definition. A CTE referenced N times runs N times. For expensive or large CTEs: use a `#temp` table to force materialization, or a table variable for small result sets. In SQL Server 2019+ with `OPTION (USE HINT('ENABLE_PARALLEL_PLAN_PREFERENCE'))`, materialization behavior can vary.
### T25 — CTE Chain Depth Exceeds 4 Levels
- **Trigger:** A `WITH` clause containing more than 4 CTEs, or CTEs that reference other CTEs forming a chain deeper than 4 levels
- **Severity:** Warning
- **Fix:** Deep CTE chains increase optimizer complexity and compile time. They also reduce readability. Refactor into: temp tables (materialized at each step), views, or a shorter set of better-named CTEs. If the depth reflects genuine business logic complexity, add inline comments explaining each step.
### T26 — Scalar Aggregate Without Explicit GROUP BY
- **Trigger:** An aggregate function (`COUNT`, `SUM`, `MAX`, `MIN`, `AVG`) in a `SELECT` list with no `GROUP BY` clause, where non-aggregate columns are also present in the SELECT — which SQL Server would reject — or where the intent of a scalar aggregate across all rows may be unintentional
- **Severity:** Info
- **Fix:** If a scalar aggregate across all rows is intended (e.g., `SELECT COUNT(*) FROM Orders`), document it. If non-aggregate columns appear alongside aggregates without GROUP BY, this is a syntax error in standard SQL — ensure SQL Server compatibility level enforces it.
### T27 — SET ROWCOUNT Usage
- **Trigger:** `SET ROWCOUNT n` statement
- **Severity:** Warning
- **Fix:** `SET ROWCOUNT` is deprecated for use with `INSERT`, `UPDATE`, and `DELETE` statements. For `SELECT`, use `TOP (@n)`. For `UPDATE` / `DELETE`, use `TOP (@n)` directly in the DML statement: `DELETE TOP (1000) FROM ...`. `SET ROWCOUNT 0` to disable is also unnecessary when using TOP.
### T28 — Missing OPTION (RECOMPILE) on High-Variance Dynamic Filter Query
- **Trigger:** A stored procedure or parameterized query that builds different effective predicates per call (e.g., optional filters using `@param IS NULL OR col = @param` patterns, or wide OR chains of nullable parameters)
- **Severity:** Info
- **Fix:** When a query's optimal plan varies significantly based on parameter values — especially with nullable "catch-all" parameters — add `OPTION (RECOMPILE)` to force per-execution plan compilation. Trade-off: recompile cost (~milliseconds) vs the cost of a bad cached plan. Evaluate with `sqlplan-review` to confirm plan sniffing symptoms (S9, N21).

---

## Security and Dynamic SQL (T29–T38, T65–T67)

Checks for SQL injection risk, privilege escalation, and dangerous server-level access.
### T29 — Dynamic SQL Built by String Concatenation
- **Trigger:** A string variable built by concatenating user-facing input (parameters, column values, or variables populated from external sources) using `+` operator, then passed to `EXEC` or `sp_executesql`: `SET @sql = 'SELECT * FROM ' + @tableName`
- **Severity:** Critical
- **Fix:** Parameterize the dynamic SQL. Values should be passed as parameters to `sp_executesql`, not concatenated. Object names (tables, columns) cannot be parameterized — validate them against `sys.tables`, `sys.columns`, or a whitelist before concatenation: `IF @tableName NOT IN ('AllowedTable1', 'AllowedTable2') RAISERROR('Invalid table', 16, 1)`. Never concatenate unvalidated strings into SQL.
### T30 — EXEC(@string) Without sp_executesql
- **Trigger:** `EXEC(@variable)` or `EXECUTE(@variable)` where `@variable` is a string — as opposed to `EXEC sp_executesql @variable, @params, @values`
- **Severity:** Critical
- **Fix:** `EXEC(@string)` cannot be parameterized. Switch to `sp_executesql` with a `@params` definition and `@values` binding. This eliminates injection risk for value-level substitutions. For object names, see T29.
### T31 — User-Controlled Input Baked Into Dynamic String
- **Trigger:** A procedure parameter (especially one typed `VARCHAR(MAX)` or `NVARCHAR(MAX)`, or named with terms like `@filter`, `@where`, `@condition`, `@sort`, `@orderby`, `@column`) used directly in string concatenation for dynamic SQL
- **Severity:** Critical
- **Fix:** If the parameter represents a value, pass it as a bound parameter to `sp_executesql`. If it represents an object name or clause fragment (ORDER BY column name, etc.), validate against an explicit whitelist or sys catalog before use. Never allow raw external strings to flow into a SQL statement.
### T32 — EXECUTE AS Without REVERT
- **Trigger:** `EXECUTE AS USER = '...'` or `EXECUTE AS LOGIN = '...'` without a corresponding `REVERT` in all exit paths (including CATCH blocks)
- **Severity:** Warning
- **Fix:** Always pair `EXECUTE AS` with `REVERT` in a `BEGIN TRY / BEGIN CATCH` structure. Failure to REVERT leaves the impersonated security context in place for the remainder of the session, potentially allowing privilege escalation.
### T33 — Hardcoded Credentials or Sensitive Literals
- **Trigger:** String literals that match patterns for passwords, connection strings, or API keys: `'password='`, `'pwd='`, `'Pass='`, `'secret='`, `'apikey='`, `'token='`, or Base64-encoded blobs longer than 64 characters used in string operations
- **Severity:** Critical
- **Fix:** Remove credentials from T-SQL source. Use Windows Authentication, Always Encrypted, or credential objects (`CREATE CREDENTIAL`). Store connection strings in application configuration, not in SQL code. Rotate any exposed credentials immediately.
### T34 — sp_executesql Called Without @params Argument
- **Trigger:** `sp_executesql @sql` called with only the `@stmt` argument and no `@params` / `@values` arguments
- **Severity:** Warning
- **Fix:** Calling `sp_executesql` without binding parameters means any values in the query are still concatenated, not parameterized. Add the `@params = N'@param1 TYPE, ...'` and the corresponding values. If no parameters are needed (fully static SQL), document why.
### T35 — OPENROWSET or OPENQUERY With Hardcoded Connection String
- **Trigger:** `OPENROWSET('provider', 'connection_string', ...)` or `OPENQUERY(linked_server, ...)` where the connection string contains credentials or server names that may be environment-specific
- **Severity:** Warning
- **Fix:** Connection strings in OPENROWSET hard-code credentials or infrastructure references into query text. Use a Linked Server object defined at the server level, or move the data access to application code where connection strings are managed via configuration.
### T36 — xp_cmdshell Reference
- **Trigger:** `xp_cmdshell` keyword anywhere in the batch
- **Severity:** Critical
- **Fix:** `xp_cmdshell` executes operating system commands from T-SQL with the SQL Server service account's privileges. This is a critical attack surface. Replace with: SQL Server Agent jobs (for scheduled OS tasks), SSIS packages (for ETL), CLR stored procedures (for file I/O with controlled permissions), or application-layer code. If `xp_cmdshell` is used in a migration or DBA script, document the specific justification and ensure `xp_cmdshell` is disabled at server level (`sp_configure 'xp_cmdshell', 0`) when not in use.
### T37 — Linked Server Query
- **Trigger:** Four-part object name: `server.database.schema.table` or `OPENQUERY(linked_server, ...)` in a DML or SELECT statement
- **Severity:** Info
- **Fix:** Linked server queries run across the network and bypass local query optimization. Ensure the linked server is needed (vs. replicating the data locally), that it uses a dedicated low-privilege login, and t

…

## Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

- **Author:** [vanterx](https://github.com/vanterx)
- **Source:** [vanterx/mssql-performance-skills](https://github.com/vanterx/mssql-performance-skills)
- **License:** MIT

Install and usage instructions live in the source repository linked above.

## Pricing

- **Free** — Free

## Security capabilities

Automated source analysis of v0.1.0 — what this tool can access:

- **Network access:** no
- **Filesystem access:** no
- **Shell / process execution:** no
- **Environment & secrets:** no
- **Dynamic code execution:** no

*"Yes" means the capability is present in the source — more access means more to trust, not that it is unsafe.*


## Versions

- **0.1.0** — security scan: flagged — Imported from the upstream source.

## Links

- Listing page: https://agentstack.voostack.com/l/skill-vanterx-mssql-performance-skills-tsql-review
- Seller: https://agentstack.voostack.com/s/vanterx
- Browse the marketplace: https://agentstack.voostack.com/browse

---
Listed on AgentStack — the marketplace for AI agent skills and MCP servers. Every listing is security-reviewed. Creators keep 70%.