SQL review agents now sit in pull request queues and often paste a rewritten statement into the same diff. That extra authority looks efficient until the new text changes lock shape, isolation, or scan family. Syntax-only checks will not catch those shifts, because the rewritten query still parses and still returns a plausible row set. The merge then ships with mixed authorship, which is a control problem rather than a model-quality complaint.
Consider a labeled incident pattern from migration review, not a claim about one private outage. An author submits a point delete that filters orders by primary key, which usually takes a narrow row lock. The agent rewrites it into a DELETE that joins an unpaid-status subquery because a style guide prefers expressive SQL. Staging stays quiet because the clone is small, and reviewers compare style instead of predicate hashes.
Production later queues checkout updates behind a wider lock than the original statement implied. The engine did what the new text asked, and the pull request still showed a green review badge. The miss was not that a model guessed a worse join; the miss was that no gate decided whether the bot may change SQL text at all.
Why rewrite authority is the real control
Most SQL review threads argue about prompts, schema dumps, or whether EXPLAIN belongs in context. Those inputs matter, yet they do not answer the ownership question that DML actually raises in review. If the agent can edit the statement, lock scope and transaction shape become shared work, and shared work is how unsafe deletes land.
Treat the model as a witness that fills a schema, or as a compiler pass that emits patches, but do not leave that choice inside the chat. The rest of this article compares those two positions with observables you can score on a clone. It then gives a numbered CI gate, a finding schema, and a decision rule that does not depend on tone.
Position A: the agent should return a patch
Teams that favor auto-rewrites treat the model as a compiler pass over SQL text. The bot can fold redundant predicates, replace unbounded SELECT * lists, and push filters before joins when the schema is stable. In a high-volume review queue, a patch reduces round trips when the finding is mechanical and the statement is read-only.
Auto-rewrite also encodes institutional style in one place instead of a wiki that nobody opens. If the organization bans NOT IN against nullable columns, the agent can emit NOT EXISTS rather than writing another comment essay. That is attractive for reporting SELECTs on a replica, where lock shape is usually not the merge risk.
The cost is authorship. Once the bot edits DML, blame for cardinality, isolation, and lock range is shared across human and model. Shared blame is a process smell, especially when the rewrite changes equality keys that also define the lock.
Position B: the agent should return evidence only
Teams that forbid patches treat the model as a witness, not a second author on the diff. The bot must emit a structured finding: statement class, tables touched, equality keys, range predicates, and a recommended human action. The original SQL remains the only text that can merge, which keeps a single writer for statements that take locks.
Evidence-only review also simplifies audit trails, because the commit still belongs to the person who typed the predicates. Reviewers can disagree with a finding without arguing about who changed the WHERE clause during the bot pass. The downside is latency, since humans must apply mechanical nits, and some style issues will survive until the next review cycle.
This position fits production DML, tenant isolation filters, and any statement whose WHERE clause is also the lock clause. It is the conservative default when the clone is incomplete or EXPLAIN output is missing.
What to score instead of helpfulness
Score each statement on four observables that a disposable clone can provide during CI. Do not score “helpfulness,” because that metric rewards silent rewrites that still parse.
- Statement class:
SELECT,INSERT,UPDATE,DELETE, DDL, or unknown after parse. - Predicate stability: did any proposed rewrite change equality keys or expand ranges?
- Plan family: Seq Scan, Index Scan, Nested Loop, Hash Join, or unknown when EXPLAIN fails.
- Authorship: is the merged text identical to the human submission for DML and DDL?
A patch that changes class 2 or class 3 should never auto-apply. A SELECT patch that only narrows the projection list may be allowed under Position A when class 3 is unchanged. DML that fails class 4 should fail the gate if you adopt Position B for writes.
| Statement class | Auto-rewrite | Required clone check | Merge text on failure |
|---|---|---|---|
| SELECT | Optional | Predicate hash and plan family match | Keep original, comment only |
| INSERT / UPDATE / DELETE | Never | EXPLAIN original only | Keep original, fail closed if EXPLAIN missing |
| DDL | Never | Parser class only | Keep original |
| Unknown | Never | Fail closed for writes | Keep original |
A numbered gate you can run in CI
The workflow below is a proposal. Label it unexecuted until you wire a real parser, a clone DSN, and your own model client.
- Parse the diff with a dialect-aware SQL parser, not a regex that splits on semicolons inside string literals.
- Classify each statement; unknown class fails closed for DML and DDL, and stays comment-only for reads.
- Ask the model for a diagnostic object only. Do not ask it to “improve” the query in the same call as the finding.
- Restore a schema-only or sanitized clone that has production-like indexes and statistics, then run
EXPLAINon the original statement. - If policy allows SELECT patches, request a rewrite in a second call, and EXPLAIN that rewrite on the same clone with the same parameters.
- Compare predicate hashes and plan families. If they diverge, drop the patch and keep the finding as evidence.
- Publish JSON on the pull request. Merged SQL stays human-authored unless the gate explicitly promotes a SELECT-only patch.
Disclosure: This article was prepared as part of MonkeyCode's product outreach. Free model access and a free server option fit steps 3–5 when you want the diagnostic call and clone-side EXPLAIN off the primary network path. Keep production credentials off that box, spend free model capacity on structured findings, and still refuse to let the agent author DML.
Artifact: finding schema, predicate gate, and clone commands
The JSON schema keeps the model from narrating. Every field is checkable in CI without reading prose.
{
"statement_id": "pr-842:migration:3",
"class": "DELETE",
"tables": ["orders"],
"equality_keys": ["orders.id"],
"range_predicates": [],
"rewrite_allowed": false,
"plan_family_original": "Index Scan",
"plan_family_rewrite": null,
"lock_note": "point delete on primary key",
"human_action": "keep original; do not widen to a status subquery"
}
Proposed Python gate (unexecuted example). It hashes normalized predicates and rejects a rewrite when DML is involved or when the hash changes.
# proposal.py — labeled example, not a production service
import hashlib
import re
DML = {"INSERT", "UPDATE", "DELETE"}
def normalize_pred(sql: str) -> str:
return re.sub(r"\s+", " ", sql.strip().lower())
def pred_hash(sql: str) -> str:
return hashlib.sha256(normalize_pred(sql).encode()).hexdigest()[:16]
def explain_family(plan: dict) -> str:
return plan.get("Node Type") or plan.get("node_type") or "unknown"
def gate(finding, original_sql, rewrite_sql, orig_plan, new_plan):
decision = {"merge_sql": original_sql, "status": "evidence_only"}
if finding["class"] in DML or finding["class"] == "DDL":
decision["reason"] = "writes_stay_human"
return decision
if not rewrite_sql:
decision["reason"] = "no_patch_requested"
return decision
same_pred = pred_hash(original_sql) == pred_hash(rewrite_sql)
same_plan = explain_family(orig_plan) == explain_family(new_plan or {})
if same_pred and same_plan and finding.get("rewrite_allowed"):
return {
"merge_sql": rewrite_sql,
"status": "select_patch_ok",
"reason": "select_plan_stable",
}
decision["reason"] = "patch_changed_pred_or_plan"
return decision
Clone-side commands stay ordinary Postgres. Point them at a disposable copy that cannot reach the primary, including through EXPLAIN of DML.
# labeled commands — CLONE_DSN must not be the primary
psql "$CLONE_DSN" -v ON_ERROR_STOP=1 <<'SQL'
EXPLAIN (FORMAT JSON, COSTS OFF)
DELETE FROM orders WHERE id = 0;
SQL
A tiny test plan you can run without a model in the loop:
- Point delete
WHERE id = 1versus rewriteWHERE id IN (SELECT id FROM orders WHERE status = 'unpaid'). The gate must keep the original text. -
SELECT * FROM orders WHERE id = 1versusSELECT id, status FROM orders WHERE id = 1. If policy allows SELECT patches and plan family matches, the gate may promote the rewrite. - Missing EXPLAIN JSON or a timed-out clone. DML fails closed; SELECT stays evidence-only and does not merge a patch.
The decision rule
Use one rule, not a prompt that asks the model to be careful with production.
If the statement is DML or DDL, the agent may not change text. If the statement is SELECT, a rewrite may merge only when predicate hash and plan family both match the original on a clone. If EXPLAIN or the model call fails, DML fails closed and SELECT stays evidence-only.
That rule turns Position A and Position B into a function of statement class, not a personality debate about agents. Current discussion around agent stacks often hides ordinary control flow behind a chatty interface. Your control flow should be this gate, with the model filling JSON fields rather than choosing who may take a lock.
Limitations and who should skip this
This gate does not prove correctness. Equal plan families can still hide partition pruning changes, row-level security gaps, or parameter sniffing on the first real bind. Predicate hashing on normalized SQL is brittle with comments, casts, and inlined views, so a dialect parser should replace the regex before you trust the hash.
Do not allow auto-rewrites when you lack a clone that resembles production indexes and statistics. Do not allow them for tenant isolation predicates, encryption wrappers, or ORM sessions that already open transactions around the statement. Do not point shared or free servers at production DSNs, even for EXPLAIN, if those statements can take locks or run functions with side effects.
Teams that need a human signature on every byte of SQL should stay on Position B for all classes. The gate then publishes evidence, and that is a complete design. Keep the decision rule in your repository either way; the useful part of the workflow is the ownership split, not the hosting choice.
Top comments (0)