# 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 optimized; mentions implicit conversions, missing indexes, SET options, or cursor usage; or wants a code review before deploying to production. No execution plan required — trigger for any T-SQL review request.

- Skill: `vanterx/tsql-review` (Agent Skill, multi-file: 8 files)
- Install (CLI): `npx skillmds@latest add vanterx/tsql-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/vanterx/tsql-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Data & Analytics
- Author: vanterx (https://skillmd.com/u/vanterx)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/vanterx/tsql-review

---


# 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 < '2025-01-01'`. This allows an index seek instead of a full scan.
### T5 — Non-Sargable Predicate — Implicit Type Coercion
- **Trigger:** A comparison where the column type and the literal or parameter type differ and SQL Server would insert an implicit `CONVERT` on the column side (e.g., `INT` column compared to an `NVARCHAR` parameter, `VARCHAR` column compared to `NVARCHAR` literal `N'value'`, `DATE` column compared to a `DATETIME` parameter)
- **Severity:** Warning
- **Fix:** Align the parameter or literal type with the column type. Declare parameters with matching types. Use explicit `CAST` on the literal rather than relying on SQL Server to cast the column. Confirm in the execution plan using the `sqlplan-review` skill (check S12/N12 implicit conversion warnings).
### T6 — Leading Wildcard LIKE
- **Trigger:** A `LIKE` predicate whose pattern starts with `%`: `LIKE '%value'` or `LIKE '%value%'`
- **Severity:** Warning
- **Fix:** Leading wildcards prevent index seeks. If full-text search is needed, use SQL Server Full-Text Search (`CONTAINS`, `FREETEXT`) or consider a computed persisted column with a suffix-reversed value. If the pattern is `LIKE '%value%'` with high selectivity, evaluate Full-Text Search or an application-side filter.
### T7 — Explicit Cursor Usage
- **Trigger:** `DECLARE ... CURSOR`, `OPEN`, `FETCH`, `CLOSE`, `DEALLOCATE` pattern
- **Severity:** Warning
- **Fix:** Replace the cursor with a set-based equivalent. Most row-by-row cursor patterns can be rewritten as: a single UPDATE with a JOIN, a recursive CTE (for hierarchical traversal), a window function (for running totals, rankings), or `STRING_AGG` / `FOR XML PATH` (for string concatenation). Cursors that update the current row (`UPDATE ... WHERE CURRENT OF`) can be rewritten as `UPDATE ... FROM ... JOIN`. If a cursor is genuinely unavoidable (DDL iteration, dynamic per-row operations), document why.
### T8 — Scalar UDF in SELECT or WHERE
- **Trigger:** A call to a user-defined scalar function (not a system function) in the `SELECT` list, `WHERE` clause, `JOIN ON`, or `ORDER BY`. Identifiable by a `schema.FunctionName()` or `dbo.fn_*()` pattern.
- **Severity:** Warning
- **Fix:** Scalar UDFs execute once per row and prevent parallelism in SQL Server 2017 and earlier. In SQL Server 2019+, Scalar UDF Inlining may handle simple functions automatically (compat level 150+). For complex UDFs or older versions: rewrite as an inline table-valued function (iTVF) and use `CROSS APPLY`, or embed the logic directly in the query. Check SQL Server version before recommending inlining as a fix.
### T9 — Correlated Subquery in SELECT List
- **Trigger:** A `SELECT` clause that contains a subquery referencing a column from the outer query: `SELECT col1, (SELECT TOP 1 x FROM T2 WHERE T2.id = outer.id) AS x`
- **Severity:** Warning
- **Fix:** A correlated subquery in the SELECT list executes once per outer row — equivalent to a cursor. Rewrite as a `LEFT JOIN` with aggregation or a window function (`FIRST_VALUE`, `MAX`). Use `OUTER APPLY` with `TOP 1` for per-row lookups when the join would change row count.
### T10 — CROSS JOIN Without Explanatory Comment
- **Trigger:** `CROSS JOIN` keyword (or a comma-separated `FROM` list with no `WHERE` join condition) with no adjacent comment explaining the intent
- **Severity:** Info
- **Fix:** If the CROSS JOIN is intentional (e.g., generating a Cartesian product for calendar rows × product rows), add a comment: `-- intentional: generates all date/product combinations`. If it is accidental (a forgotten JOIN condition), add the condition.
### T11 — OR Condition in JOIN Predicate
- **Trigger:** A `JOIN ... ON` clause that uses `OR` between join conditions: `ON a.id = b.id OR a.alt_id = b.id`
- **Severity:** Info
- **Fix:** OR in a JOIN predicate often prevents nested-loop seeks and forces a hash or merge join scanning both inputs. Rewrite as a `UNION ALL` of two separate joins, one for each condition. This allows independent seek paths.
### T12 — Function on Indexed Column in JOIN ON Clause
- **Trigger:** A function wrapping a column in a `JOIN ... ON` clause: `ON CAST(a.col AS INT) = b.col` or `ON YEAR(a.date) = b.year`
- **Severity:** Warning
- **Fix:** Same as T4 but in join context. Ensure the join column is bare and the function is applied to the other side, or store the pre-computed value as a persisted computed column.
### T13 — ISNULL or COALESCE on Indexed Column in WHERE
- **Trigger:** `ISNULL(col, substitute)` or `COALESCE(col, substitute)` in a `WHERE` clause where the first argument is a column reference
- **Severity:** Warning
- **Fix:** This pattern makes the predicate non-sargable. Rewrite as: `WHERE (col = @param OR (col IS NULL AND @param = substitute))`. This preserves seek ability and handles both cases explicitly.
### T14 — Missing TOP or FETCH NEXT (Unbounded Result Set)
- **Trigger:** A `SELECT` statement that returns rows to a caller or application (not used as a subquery, CTE, or INSERT source) with no `TOP`, `FETCH NEXT`, or explicit pagination
- **Severity:** Info
- **Fix:** If this is a reporting query with intentionally unbounded results, document it. For API-facing or application-facing queries, add `TOP (@n)` or `OFFSET 0 ROWS FETCH NEXT @pageSize ROWS ONLY` with a deterministic `ORDER BY`.
### T15 — DISTINCT Without Aggregation Intent
- **Trigger:** `SELECT DISTINCT` where no aggregation or deduplication is clearly needed — often masking a bad JOIN that inflates row count
- **Severity:** Info
- **Fix:** Investigate why rows are duplicated before adding DISTINCT. A JOIN producing duplicates usually indicates a missing aggregation or a one-to-many relationship that should use EXISTS/IN instead of a direct join. `SELECT DISTINCT` is a symptom, not a fix.

---

## Correctness and Logic (T16–T28, T56–T64)

Checks for logic errors that produce wrong results or unreliable behavior, often silently.
### T16 — NULL Comparison Using = NULL
- **Trigger:** A predicate of the form `col = NULL` or `col <> 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 that the query is selective enough to minimize data transfer. Flag for security review of linked server credentials.
### T38 — Missing Schema Prefix on Object Name
- **Trigger:** A table, view, function, or procedure reference without a schema prefix: `FROM Orders` instead of `FROM dbo.Orders`, or `EXEC GetOrder` instead of `EXEC dbo.GetOrder`
- **Severity:** Info
- **Fix:** Unqualified names are resolved by SQL Server using the calling user's default schema first, then dbo. This causes plan cache pollution (different users → different plans for the same object), and can silently execute the wrong object if schema-shadowing occurs. Always use two-part names: `schema.ObjectName`.

---

## Deprecated and Non-Idiomatic Syntax (T39–T45, T68–T73)

Checks for syntax that is removed, deprecated, or diverges from SQL Server best practice.
### T39 — Deprecated Outer Join Syntax
- **Trigger:** `*=` or `=*` join operators in a `WHERE` clause (old Sybase-style outer join syntax)
- **Severity:** Critical
- **Fix:** This syntax was removed in SQL Server 2012 (11.x) and is invalid at compatibility level 90 (SQL Server 2005) and above. Rewrite using ANSI `LEFT JOIN` or `RIGHT JOIN` syntax. Example: `WHERE a.id *= b.id` → `FROM a LEFT JOIN b ON a.id = b.id`.
### T40 — Non-ANSI GROUP BY Behavior
- **Trigger:** A `SELECT` statement with a `GROUP BY` clause where columns in the SELECT list are neither in the GROUP BY nor wrapped in an aggregate function, and the query is running at a compatibility level that permits this (SQL Server 2000 compatibility / `GROUP BY ALL`)
- **Severity:** Warning
- **Fix:** Remove `GROUP BY ALL` and include all non-aggregated columns in the GROUP BY clause. `GROUP BY ALL` is deprecated and produces undefined behavior for non-participating groups (it includes them with NULL aggregate values). Rewrite to use ANSI-compliant GROUP BY or replace with a window function.
### T41 — RAISERROR Instead of THROW
- **Trigger:** `RAISERROR` statement
- **Severity:** Info
- **Fix:** The modern `RAISERROR(msg, severity, state)` syntax is **not** on Microsoft's deprecated-features list — only the legacy comma-less integer-string form (`RAISERROR 50001 'message'`, tracked as "Oldstyle RAISERROR") is formally deprecated. That said, MS Learn recommends `THROW` (SQL Server 2012+) for new development. Two distinct forms: (1) Bare `THROW;` inside a CATCH block re-raises the caught exception with its original error number and severity — use this to propagate errors up the call stack. (2) `THROW error_number, message, state` raises a new exception; the severity is always 16 (the caller cannot change it). Replace `RAISERROR('msg', 16, 1)` with `THROW 50001, N'msg', 1` for new application errors; in CATCH blocks, use bare `THROW;` instead of `RAISERROR(@ErrorMessage, @ErrorSeverity, @ErrorState)`.
### T42 — GETDATE() Where SYSDATETIME() Preferred
- **Trigger:** `GETDATE()` function call in a context where higher precision or UTC time is appropriate
- **Severity:** Info
- **Fix:** `GETDATE()` returns `DATETIME` (3.33ms precision, local server time). Prefer `SYSDATETIME()` for `DATETIME2(7)` precision, or `SYSUTCDATETIME()` for UTC time. For audit timestamps, always use UTC. For compatibility with legacy `DATETIME` columns, `GETDATE()` remains acceptable — flag only when a new timestamp column is being designed.
### T43 — INSERT Without Column List
- **Trigger:** `INSERT INTO table VALUES (...)` with no explicit column list
- **Severity:** Warning
- **Fix:** `INSERT ... VALUES` without a column list assumes values match the physical column order. A schema change (adding, removing, or reordering a column) silently breaks the INSERT or inserts values into the wrong columns. Always use `INSERT INTO table (col1, col2, ...) VALUES (...)`.
### T44 — SET ANSI_NULLS OFF or SET QUOTED_IDENTIFIER OFF
- **Trigger:** `SET ANSI_NULLS OFF` or `SET QUOTED_IDENTIFIER OFF` statement
- **Severity:** Warning
- **Fix:** `SET ANSI_NULLS OFF` is **deprecated** (it is on Microsoft's deprecated-features list, and starting with SQL Server 2017 ANSI_NULLS is always ON); it changes `= NULL` comparison semantics (T16). `SET QUOTED_IDENTIFIER OFF` is **not** on the deprecated list, but it is strongly discouraged: it disables standard double-quote identifier delimiting (treating `"..."` as a string literal) and is non-ISO. Both `QUOTED_IDENTIFIER` and `ANSI_NULLS` must be ON for indexed views, indexes on computed columns, filtered indexes, and natively compiled objects. Remove these SET statements and fix any code that depended on them.
### T45 — Temporary Table Created Without Explicit Column Definition
- **Trigger:** `SELECT ... INTO #tempTable FROM ...` (implicit column definition) rather than `CREATE TABLE #tempTable (col1 TYPE, ...)` followed by `INSERT INTO`
- **Severity:** Info
- **Fix:** `SELECT INTO` infers column names and types from the source expression. This is fragile: a source column rename or type change silently changes the temp table schema. For temp tables that are accessed more than once or share structure with permanent tables, prefer an explicit `CREATE TABLE #name (...)` with defined types. `SELECT INTO` is acceptable for quick ad-hoc materializtion.

---

## Performance Smells (T46–T50, T74–T78)

Checks for patterns that are likely to degrade performance at scale, even when syntactically correct.
### T46 — Table Variable Used for Potentially Large Data
- **Trigger:** `DECLARE @table TABLE (...)` used in contexts suggesting large row counts: populated from a join across large tables, used as a parameter accumulator in a loop, referenced in a query without cardinality hints, or when the surrounding code suggests > 1,000 rows
- **Severity:** Warning
- **Fix:** Table variables have no column statistics. Before SQL Server 2019, the optimizer estimated **1 row** regardless of actual content, causing bad join plans for large table variables. **Starting with SQL Server 2019 (compatibility level 150), Table Variable Deferred Compilation** defers the first compile until the table variable is populated, so the optimizer uses the **actual** row count for cardinality (it still lacks column-level statistics, so skew can still mislead it). So this check is most impactful below compat 150; on 2019+ the 1-row trap is mitigated but column-stat blindness remains. Use a `#temp` table when you need real statistics, indexes, or parallel plans. Exception: table variables are appropriate for small lookup sets (< 100 rows) or as a TVP.
### T47 — String Functions on Potentially Large Rowsets
- **Trigger:** `STRING_SPLIT`, `CHARINDEX`, `SUBSTRING`, `PATINDEX`, `REPLACE`, or `STUFF` called in a `FROM` clause, `WHERE` clause, or `SELECT` list against a large table (inferred from table names or surrounding joins)
- **Severity:** Info
- **Fix:** String functions are CPU-intensive per row. For `STRING_SPLIT` as a JOIN source, ensure the split list is small. For `CHARINDEX`/`PATINDEX` in WHERE clauses, consider adding a computed persisted column with an index. For aggregation using `STRING_AGG` or `FOR XML PATH`, ensure the input rowset is pre-filtered.
### T48 — Deeply Nested Scalar Subqueries
- **Trigger:** A scalar subquery nested 3 or more levels deep: a subquery inside a subquery inside a subquery
- **Severity:** Warning
- **Fix:** Deep subquery nesting compounds execution cost — each level may execute once per row of its parent. Refactor using: CTEs to name each subquery level, window functions to replace per-row lookups, or a series of temp tables to materialize intermediate results. Deep nesting also reduces readability and increases maintenance risk.
### T49 — Pagination Without Deterministic Sort Key
- **Trigger:** `OFFSET n ROWS FETCH NEXT m ROWS ONLY` or `ROW_NUMBER() OVER (ORDER BY ...)` pagination where the `ORDER BY` does not include a unique key column (e.g., primary key or unique column)
- **Severity:** Warning
- **Fix:** Without a unique sort key, the ordering of rows with equal sort values is non-deterministic. A user paging through results may see the same row on two pages or skip rows entirely when the underlying data changes between pages. Always include a unique column (e.g., `ORDER BY CreatedDate DESC, OrderId ASC`) as a tiebreaker.
### T50 — Implicit Collation or Type Conversion in Comparison
- **Trigger:** A comparison or JOIN between columns with different collations (e.g., `Latin1_General_CI_AS` vs `SQL_Latin1_General_CP1_CI_AS`) or between string columns of different types (`VARCHAR` vs `NVARCHAR`) without explicit COLLATE or CAST
- **Severity:** Warning
- **Fix:** Collation mismatches prevent index usage and can cause errors at runtime if collation compatibility is not met. Resolve by: aligning column collations (ALTER TABLE ALTER COLUMN), adding an explicit `COLLATE` clause in the query, or casting both sides to the same type. Confirm no index seeks are blocked using `sqlplan-review` (check T5 / N12).

### T51 — NOT IN With Nullable Subquery
- **Trigger:** `NOT IN (SELECT col FROM ...)` where `col` is nullable — no `WHERE col IS NOT NULL` filter inside the subquery, or the column is not defined NOT NULL
- **Severity:** Critical
- **Fix:** Replace with `NOT EXISTS (SELECT 1 FROM T WHERE T.col = outer.col)`, which handles NULLs correctly. If keeping `NOT IN`, add `WHERE col IS NOT NULL` inside the subquery. Three-valued logic causes the entire `NOT IN` to return zero rows whenever the subquery returns any NULL.
### T52 — Division by Zero Without NULLIF Guard
- **Trigger:** A division expression (`/`) where the denominator is a column reference, variable, or expression that could evaluate to zero, with no `NULLIF(denominator, 0)` or `CASE WHEN denominator = 0 THEN NULL END` guard
- **Severity:** Warning
- **Fix:** Wrap the denominator: `numerator / NULLIF(denominator, 0)`. Returns NULL instead of raising error 8134 when the denominator is zero.
### T53 — TOP Without ORDER BY
- **Trigger:** `SELECT TOP (n)` or `SELECT TOP n` in a top-level or API-facing SELECT statement with no `ORDER BY` clause
- **Severity:** Warning
- **Fix:** Add an `ORDER BY` clause that determines which rows qualify as "top". If random sampling is intended, use `ORDER BY NEWID()` and add a comment documenting the intent.
### T54 — COUNT(\*) > 0 Instead of EXISTS
- **Trigger:** `IF (SELECT COUNT(*) FROM T WHERE ...) > 0`, `WHERE (SELECT COUNT(*) FROM T WHERE ...) > 0`, or a subquery `COUNT(*) > 0` check used solely to test for the existence of rows (not to use the count value)
- **Severity:** Info
- **Fix:** Replace with `IF EXISTS (SELECT 1 FROM T WHERE ...)` or `WHERE EXISTS (...)`. `EXISTS` short-circuits at the first matching row; `COUNT(*)` must scan all matching rows.
### T55 — VARCHAR/NVARCHAR Implicit Promotion in String Concatenation
- **Trigger:** A `+` string concatenation expression that mixes `VARCHAR` literals or variables with `NVARCHAR` literals (`N'...'`) or `NVARCHAR` variables, causing implicit promotion of the whole expression to `NVARCHAR`
- **Severity:** Warning
- **Fix:** Use `NVARCHAR` consistently for all variables and literals when building dynamic SQL. Mixing `VARCHAR` and `NVARCHAR` in a concatenation doubles memory consumption for the `VARCHAR` operand and may silently corrupt characters above code-point 127.

---

### T56 — @@IDENTITY Instead of SCOPE\_IDENTITY()
- **Trigger:** `@@IDENTITY` used to retrieve the last-inserted identity value after an `INSERT` statement
- **Severity:** Warning
- **Fix:** Replace `@@IDENTITY` with `SCOPE_IDENTITY()`. `@@IDENTITY` returns the last identity inserted in the session across all scopes including triggers; `SCOPE_IDENTITY()` returns the value from the current scope only. For multiple-row inserts, use `OUTPUT INSERTED.id INTO @ids`.
### T57 — @@ROWCOUNT Read After Statement That Resets It
- **Trigger:** A `@@ROWCOUNT` check that is not the statement immediately following the DML whose count is needed — any `SET`, `DECLARE`, `IF`, `SELECT @var = ...`, `PRINT`, or other non-DML statement appears between the DML and the `@@ROWCOUNT` read
- **Severity:** Warning
- **Fix:** Capture `@@ROWCOUNT` immediately after the DML: `SET @rowsAffected = @@ROWCOUNT;`. Intervening statements change it in different ways — a scalar assignment such as `SET @v = 1` or `SELECT @v = <constant>` (no FROM) sets `@@ROWCOUNT` to **1**, a `SELECT @v = col FROM t` sets it to **that query's row count**, and statements like `USE`/`SET <option>` reset it to **0** — so never read it after anything but the DML whose count you need.
### T58 — Recursive CTE Without MAXRECURSION
- **Trigger:** A recursive CTE (a `WITH` clause where the CTE references itself in its recursive member) without `OPTION (MAXRECURSION n)` on the outermost SELECT
- **Severity:** Warning
- **Fix:** Add `OPTION (MAXRECURSION n)` where `n` reflects the maximum expected hierarchy depth. The default is 100; hierarchies deeper than 100 levels fail with error 530. Use `OPTION (MAXRECURSION 0)` only with an explicit depth-counter guard in the recursive member to prevent infinite loops.
### T59 — MERGE Without HOLDLOCK (Race Condition)
- **Trigger:** A `MERGE` statement where the target table does not have a `WITH (HOLDLOCK)` or `WITH (SERIALIZABLE)` hint and the statement is not inside a serializable transaction
- **Severity:** Critical
- **Fix:** Add `WITH (HOLDLOCK)` to the MERGE target: `MERGE dbo.Target WITH (HOLDLOCK) AS t USING @source AS s ON ...`. Without it, concurrent sessions can both pass the NOT MATCHED check and both attempt the INSERT, causing a primary key violation (error 2627) or duplicate row.
### T60 — DATEDIFF as Non-Sargable Date Range Filter
- **Trigger:** `DATEDIFF(part, column, expression)` or `DATEDIFF(part, expression, column)` in a `WHERE`, `HAVING`, or `JOIN ON` clause where one argument is a table column reference
- **Severity:** Warning
- **Fix:** Rewrite as a range predicate on the bare column: `WHERE col >= DATEADD(part, -n, reference_date)`. Example: `WHERE DATEDIFF(DAY, OrderDate, GETDATE()) <= 30` → `WHERE OrderDate >= DATEADD(DAY, -30, CAST(GETDATE() AS DATE))`.
### T61 — BETWEEN on DATETIME With Date-Only Boundaries
- **Trigger:** A `BETWEEN` predicate on a `DATETIME` or `DATETIME2` column where both boundaries are date-only literals (`'YYYY-MM-DD'`), parameters typed as `DATE`, or expressions truncated to midnight: e.g., `col BETWEEN '2024-01-01' AND '2024-01-31'`
- **Severity:** Warning
- **Fix:** Replace with a half-open range: `WHERE col >= '2024-01-01' AND col < '2024-02-01'`. `BETWEEN '2024-01-01' AND '2024-01-31'` excludes all times on January 31st after midnight.
### T62 — SELECT Variable Assignment — Silent Prior-Value Retention
- **Trigger:** `SELECT @var = col FROM T WHERE ...` used to fetch a single scalar value, without an explicit `SET @var = NULL` before the SELECT or a `@@ROWCOUNT` check after, in contexts where the caller expects `@var` to be NULL when no rows match
- **Severity:** Info
- **Fix:** Use `SET @var = (SELECT col FROM T WHERE ...)` if NULL-on-no-rows is the intended semantics, or initialize `@var = NULL` before the SELECT. Unlike `SET`, `SELECT @var = col` leaves `@var` at its previous value when the query returns no rows.
### T63 — ISNUMERIC() for Numeric Type Validation
- **Trigger:** `ISNUMERIC(expression)` in a WHERE clause, IF condition, or CASE expression to validate that a string can be safely cast to a numeric type
- **Severity:** Warning
- **Fix:** Replace with `TRY_CAST(expression AS target_type) IS NOT NULL` or `TRY_CONVERT(target_type, expression) IS NOT NULL`. `ISNUMERIC()` returns 1 for `'+'`, `'-'`, `'.'`, `','`, `'$'`, and `'E'` — all of which fail `CAST(... AS INT)` or `CAST(... AS DECIMAL)`.
### T64 — Output Parameter Not Initialized in All Code Paths
- **Trigger:** A stored procedure or function with an `OUTPUT` parameter that is not assigned a value in one or more code paths through the procedure body (assignment inside an IF branch that may not execute, or only inside the TRY block)
- **Severity:** Warning
- **Fix:** Initialize all `OUTPUT` parameters at the top of the procedure body before any branching: `SET @outParam = NULL;`. Note: assigning a default in the procedure signature (`@outParam INT = 0 OUTPUT`) does not count — the assignment must be inside the body.

---

### T65 — Dangerous OLE Automation and Registry Extended Procedures
- **Trigger:** Any reference to: `sp_OACreate`, `sp_OAMethod`, `sp_OADestroy`, `sp_OAGetProperty`, `sp_OASetProperty`, `sp_OAGetErrorInfo`, `xp_regread`, `xp_regwrite`, `xp_regdeletevalue`, `xp_regdeletekey`, `xp_servicecontrol`
- **Severity:** Critical
- **Fix:** Remove entirely. Replace with SQL Server Agent jobs, SSIS packages, PowerShell via Agent, or application-layer code. These allow arbitrary COM object execution, registry read/write, and service control under the SQL Server service account — same risk category as `xp_cmdshell` (T36). Verify disabled server-wide: `EXEC sp_configure 'Ole Automation Procedures'` should return 0.
### T66 — QUOTENAME Misapplied to Values Instead of Identifiers
- **Trigger:** `QUOTENAME(@var)` or `QUOTENAME(col)` where the argument represents a data value (a filter condition, status code, search term) rather than a SQL Server object name (table, column, schema name)
- **Severity:** Critical
- **Fix:** `QUOTENAME` adds square brackets and is safe only for identifier injection. It does not sanitize values — a value like `]; DROP TABLE T; --` remains injectable after `QUOTENAME`. Pass all values as bound parameters via `sp_executesql @params`. Use `QUOTENAME` only for validated object-name tokens confirmed against `sys.tables`/`sys.columns`.
### T67 — Dynamic SQL Built and Executed Inside WHILE Loop
- **Trigger:** A `WHILE` loop or cursor body that contains both dynamic SQL construction (`SET @sql = ...`) and execution (`EXEC(@sql)` or `EXEC sp_executesql @sql`) within the same loop iteration
- **Severity:** Warning
- **Fix:** Mov

…(truncated)
