Skill: Review SQL

Purpose

Review SQL and query-related code for language and query conventions only. Cover injection and parameterization, indexing and execution-plan concerns, transactions and isolation, NULL and unique constraints, dialect portability, large-table and paging patterns, and sensitive columns and permissions. Emit a findings list in the standard format for aggregation. Do not define scope or perform full security/architecture review; injection is in scope here as an SQL-specific concern, but broader security is for review-security.

Core Objective

Primary goal: Produce a SQL-focused findings list covering injection/parameterization, indexing, transactions, NULL/constraints, dialect portability, paging patterns, and sensitive column access for the given code scope.

Success Criteria (ALL must be met):

1. ✅ SQL-only scope: Only SQL and query conventions are reviewed; no scope selection, full security, or architecture analysis performed
2. ✅ All seven SQL dimensions covered: Injection/parameterization, indexing/execution plan, transactions/isolation, NULL/unique constraints, dialect/portability, large table/paging patterns, and sensitive columns/permissions are assessed where relevant
3. ✅ Findings format compliant: Every finding carries every element findings-list §5.1 requires, with category language-sql
4. ✅ Critical injection issues flagged: SQL injection patterns (string concatenation, interpolation with user input) are marked as critical severity
5. ✅ Location-precise references: All findings reference specific file:line or query identifier locations

Acceptance Test: Does the output contain a SQL findings list covering all relevant query dimensions, with injection risks marked critical and specific location references for every finding?

Scope Boundaries

This skill handles:

• SQL injection via string concatenation or interpolation — parameterized queries and prepared statements
• Indexing gaps for WHERE/JOIN/ORDER BY columns
• Transaction boundaries, isolation levels, deadlock risk, long-running transactions
• NULL handling in comparisons/aggregates, unique constraints, NOT NULL correctness
• Database dialect portability (LIMIT/OFFSET/FETCH, date functions, vendor-specific syntax)
• Large table full scans, paging strategies (keyset vs OFFSET)
• Sensitive column exposure in SELECT, least-privilege role usage

This skill does NOT handle:

• Scope selection — scope is provided by the caller
• Broader security analysis (beyond SQL injection) — use review-security
• Architecture analysis — use review-architecture
• Full orchestrated review — use orchestrate-code-review

Handoff point: When all SQL findings are emitted, hand off to orchestrate-code-review for aggregation. For broader security concerns (auth, crypto, config), redirect to review-security.

Use Cases

• Orchestrated review: Used as the language step when orchestrate-code-review runs for projects that include SQL (.sql files, embedded SQL, or ORM-generated SQL).
• SQL-only review: When the user wants only query correctness, performance, and safety checked.
• Migration or portability: Check dialect-specific constructs and portability across databases.

When to use: When the code under review includes SQL (raw .sql, embedded in code, or ORM-generated). Scope (diff vs paths) is determined by the caller or user.

Behavior

Scope of this skill

• Analyze: SQL and query logic in the given scope (files, snippets, or diff). Accept .sql files, embedded SQL in application code, or ORM-generated SQL when visible.
• Do not: Decide scope (diff vs codebase); do not perform full application security or architecture review. Focus on SQL/query dimension.

Review checklist (SQL dimension only)

1. Injection and parameterization: No string concatenation or interpolation for user input in SQL; use parameterized queries or prepared statements; avoid dynamic SQL from untrusted input.
2. Indexing and execution plan: Queries that filter or join on unindexed columns; SELECT * on large tables; missing indexes for WHERE/JOIN/ORDER BY.
3. Transactions and isolation: Appropriate transaction boundaries; isolation level and locking; avoid long-running transactions; deadlock risk.
4. NULL and unique constraints: Handling of NULL in comparisons and aggregates; unique constraints and duplicate handling; NOT NULL where appropriate.
5. Dialect and portability: Database-specific syntax (e.g. LIMIT vs OFFSET/FETCH, date functions) and portability if multi-DB support is required.
6. Large tables and paging: Full scans on large tables; paging (keyset vs OFFSET) and scalability.
7. Sensitive columns and permissions: Sensitive data in SELECT; least-privilege and role usage in SQL (where visible).

Tone and references

• Professional and technical: Reference specific locations (file:line or query identifier). Emit findings carrying every element findings-list §5.1 requires.

Input & Output

Input

• Code scope: Files or snippets containing SQL (e.g. .sql files, code with embedded SQL, or ORM-generated SQL when available). Provided by the user or scope skill.

Output

• Emit zero or more findings in the format defined in specs/findings-list.md, with Category language-sql.
• Category for this skill is language-sql.

Restrictions

Hard Boundaries

• Do not perform scope selection or full security/architecture review. Stay within SQL and query conventions.
• Do not emit a finding without a precise location. When a suggestion is included, it must be actionable; follow the canonical Findings Spec for required elements.
• Do not assume a specific database vendor unless stated; note dialect when relevant.

Skill Boundaries

Do NOT do these (other skills handle them):

• Do NOT select or define the code scope — scope is determined by the caller or orchestrate-code-review
• Do NOT perform broad security analysis beyond SQL injection — use review-security
• Do NOT perform architecture analysis — use review-architecture
• Do NOT review non-SQL code for SQL conventions (SQL injection in application code should be flagged by review-security)

When to stop and hand off:

• When all SQL findings are emitted, hand off to orchestrate-code-review for aggregation
• When the user needs broader security analysis (auth, crypto, config), redirect to review-security
• When the user needs a full review (scope + language + cognitive), redirect to orchestrate-code-review

Self-Check

Core Success Criteria

• SQL-only scope: Only SQL and query conventions are reviewed; no scope selection, full security, or architecture analysis performed
• All seven SQL dimensions covered: Injection/parameterization, indexing/execution plan, transactions/isolation, NULL/unique constraints, dialect/portability, large table/paging patterns, and sensitive columns/permissions are assessed where relevant
• Findings format compliant: Every finding carries every element findings-list §5.1 requires, with category language-sql
• Critical injection issues flagged: SQL injection patterns (string concatenation, interpolation with user input) are marked as critical severity
• Location-precise references: All findings reference specific file:line or query identifier locations

Process Quality Checks

• Was only the SQL/query dimension reviewed (no scope/architecture beyond query design)?
• Are parameterization, indexing, transactions, NULL/constraints, and portability covered where relevant?
• Does every finding carry every element findings-list §5.1 requires, with category=language-sql?
• Are issues referenced with file:line or query identifier?

Acceptance Test

Does the output contain a SQL findings list covering all relevant query dimensions, with injection risks marked critical and specific location references for every finding?

Examples

Example 1: String concatenation in query

• Input: Query built with string concatenation including user input.
• Expected: Emit a critical finding for SQL injection; suggest parameterized query or prepared statement. Category = language-sql.

Example 2: Large table without paging

• Input: SELECT * FROM large_table ORDER BY id without LIMIT or paging.
• Expected: Emit finding for performance and scalability; suggest paging (e.g. keyset or OFFSET/FETCH) and avoid SELECT * if not needed. Category = language-sql.

Edge case: ORM-generated SQL

• Input: Only application code using an ORM; generated SQL not visible.
• Expected: Review any raw SQL or query builders in the code; if no SQL is visible, state that and skip or report "No SQL to review in scope." Do not invent SQL.