opencode-build/skills/sql-review/SKILL.md

name, description

name	description
sql-review	Use when performing a static review of a generated PySpark SQL string before it is executed. Triggers include "review this SQL", "SQL 评审", "静态审查", "risk level", "check join cardinality", "数据膨胀", "重复聚合", "分区裁剪". Consumes a SQL string plus the SQL Context that produced it, and emits a risk-graded review (HIGH / MEDIUM / LOW) with concrete fixes. Does NOT modify the SQL — it reports findings.

SQL Review

Overview

Static, pre-execution review of a generated PySpark SQL string. The reviewer does not rewrite SQL — it emits findings, severity, and suggested fixes. The author of the SQL decides what to act on.

Core principle: review against the SQL Context, not against vibes. Every field, alias, and join key referenced in the SQL must exist in the SQL Context that produced it. If it does not, that is itself a HIGH finding (the SQL bypassed the contract).

When to Use

Use when:

• A SQL string has been generated from a sql_context and is about to be executed
• The user pastes a SQL string and asks for a review, optimization, or risk assessment
• A code change adds a new spark.sql(...) call to a PySpark script

Do NOT use when:

• The SQL is not Spark SQL (e.g. plain Postgres, HiveQL outside Spark) — at minimum replace Spark-specific checks with the relevant engine's equivalents, but flag it
• The SQL is a one-off interactive query that won't be reused — the user can opt out

Inputs

1. The SQL string to review
2. The SQL Context that the SQL was generated from (or, if absent, an explicit acknowledgement that the SQL is "untrusted / ad-hoc")

If the SQL Context is missing, raise a single HIGH finding: "SQL has no associated SQL Context; downstream skills cannot guarantee field/join correctness." Then continue with structural checks.

Severity Model

Level	Meaning	Action
HIGH	Will produce wrong numbers, fail to run, or violate policy	Block execution. Must fix.
MEDIUM	Likely correct, but inefficient, fragile, or risky at scale	Fix before production. OK in dev.
LOW	Style / best-practice nit	Optional fix.

Emit findings in this order: HIGH → MEDIUM → LOW. A single HIGH finding makes the overall review verdict FAIL.

Workflow

digraph sql_review {
    "Parse SQL (logical, not just regex)" [shape=box];
    "Cross-check aliases, columns, join keys against SQL Context" [shape=box];
    "Detect multi-statement or forbidden verbs (guardrail)" [shape=box];
    "Analyze join cardinality & inflation risk" [shape=box];
    "Detect repeated aggregation on same measure" [shape=box];
    "Detect full table scan & missing partition filter" [shape=box];
    "Verify GROUP BY matches SELECT" [shape=box];
    "Check null handling on join keys & measures" [shape=box];
    "Check field types vs operations" [shape=box];
    "Aggregate findings into verdict" [shape=box];

    "Parse SQL (logical, not just regex)" -> "Cross-check aliases, columns, join keys against SQL Context";
    "Cross-check aliases, columns, join keys against SQL Context" -> "Detect multi-statement or forbidden verbs (guardrail)";
    "Detect multi-statement or forbidden verbs (guardrail)" -> "Analyze join cardinality & inflation risk";
    "Analyze join cardinality & inflation risk" -> "Detect repeated aggregation on same measure";
    "Detect repeated aggregation on same measure" -> "Detect full table scan & missing partition filter";
    "Detect full table scan & missing partition filter" -> "Verify GROUP BY matches SELECT";
    "Verify GROUP BY matches SELECT" -> "Check null handling on join keys & measures";
    "Check null handling on join keys & measures" -> "Check field types vs operations";
    "Check field types vs operations" -> "Aggregate findings into verdict";
}

Checks (run all)

1. Reference integrity — every <alias>.<column> in the SQL must exist in the SQL Context. Mismatches are HIGH.
2. Join key match — every ON predicate must match a frozen join from the SQL Context. Invented joins are HIGH.
3. Forbidden verbs / multi-statement — defer to pyspark-sql-guardrails. Any DDL/DML/maintenance verb, or any ;-stacked statement, is HIGH.
4. Join cardinality risk — for every join, if the SQL Context marks cardinality as 1:N or N:N and there is no dedup, raise MEDIUM (1:N) or HIGH (N:N).
5. Repeated aggregation — if the same measure is aggregated twice (e.g. once in a subquery, once outside) without intentional re-aggregation, raise HIGH.
6. Partition / time filter — for any fact table with a known time field, missing WHERE on that field is HIGH (full scan risk).
7. GROUP BY completeness — every selected non-aggregated column must be in GROUP BY. Mismatch is HIGH.
8. Null handling — nullable join keys with INNER JOIN silently drop rows (MEDIUM); nullable measures aggregated without COALESCE can produce NULL that breaks downstream WHERE ... > 0 (MEDIUM).
9. Type mismatch — string columns used in numeric aggregation without cast (MEDIUM); date string compared to TIMESTAMP literal (MEDIUM).
10. Spark best practice — COUNT(*) vs COUNT(1) is a style nit (LOW); SELECT * in production is MEDIUM; missing DISTRIBUTE BY / CLUSTER BY for skewed joins is LOW to MEDIUM.

Output Contract

sql_review:
  verdict: <PASS | FAIL>
  highest_severity: <HIGH | MEDIUM | LOW>
  summary: "<one-sentence summary>"
  findings:
    - id: F1
      severity: HIGH
      check: reference_integrity
      location: "<line/column or SQL fragment>"
      message: "<what is wrong>"
      fix: "<concrete suggested change>"
    - id: F2
      severity: MEDIUM
      check: join_cardinality
      location: "JOIN customer_info"
      message: "1:N join without pre-aggregation; risk of row inflation"
      fix: "Aggregate loan_order by customer_id before joining"
  notes:
    - "SQL Context provided: yes (sql_context.aliases = [...])"
    - "Reviewed by: sql-review v1"

Forbidden Behaviors

Rationalization	Reality
"The SQL looks fine, no need to cross-check the context"	A column can look fine and still not exist in the source table. Cross-check is mandatory.
"I'll fix the SQL while reviewing"	The reviewer reports. The author fixes. Separation of concerns.
"No SQL Context was provided, so I'll skip field checks"	Missing context is itself a HIGH finding. Never silently skip.
"It's just a dev query, skip partition check"	The check is cheap. Skipping it produces silent full scans.
"1:N joins are normal, no risk"	1:N is fine if the grain is preserved. The reviewer must verify, not assume.
"Regex is enough to check GROUP BY"	GROUP BY membership is structural. Use a parser, not regex.

Completion Criteria

A review is complete when:

• All 10 checks have been evaluated
• Every HIGH finding has a fix
• verdict is PASS iff highest_severity is MEDIUM or LOW
• The output YAML can be pasted into a PR comment without edits