feat: add visual explain plan with AI analysis#128
Conversation
prevent EXPLAIN on DDL and show localized error
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| // EXPLAIN ANALYZE — MySQL 8.0.18+ text tree with estimated + actual data | ||
| if analyze && caps.supports_explain_analyze { | ||
| let mut conn = pool.acquire().await.map_err(|e| e.to_string())?; | ||
| let analyze_sql = format!("EXPLAIN ANALYZE {}", query); |
There was a problem hiding this comment.
CRITICAL: SQL Injection vulnerability. The user-supplied query is directly interpolated into SQL without parameterization. While is_explainable_query provides basic filtering, it's not sufficient protection.
This pattern appears in multiple locations in this file (lines 1103, 1136, 1169).
| } | ||
|
|
||
| let explain_sql = if analyze { | ||
| format!("EXPLAIN (FORMAT JSON, ANALYZE, BUFFERS) {}", query) |
There was a problem hiding this comment.
CRITICAL: SQL Injection vulnerability. The user-supplied query is directly interpolated into SQL without parameterization. While there's upstream validation, direct string interpolation into SQL is unsafe.
Same issue at line 1035.
| let pool = get_sqlite_pool(params).await?; | ||
| let mut conn = pool.acquire().await.map_err(|e| e.to_string())?; | ||
|
|
||
| let explain_sql = format!("EXPLAIN QUERY PLAN {}", query); |
There was a problem hiding this comment.
CRITICAL: SQL Injection vulnerability. The user-supplied query is directly interpolated into SQL without parameterization.
| </div> | ||
| {entries.map(([name, value], i) => ( | ||
| <div | ||
| key={`${name}-${i}`} |
There was a problem hiding this comment.
WARNING: React key collision risk. Using name-i as key can still cause collisions if the same property name appears twice. Consider using a more unique key like node.id-name-i.
| Ok(plan) | ||
| } | ||
| Ok(Err(e)) => { | ||
| log::error!("Explain query failed: {}", e); |
There was a problem hiding this comment.
SUGGESTION: Double error handling - error is logged here and then returned to caller who may also log it. Consider removing the log::error! here to avoid duplicate logging.
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address CRITICAL issues before merge Overview
Issue Details (click to expand)CRITICAL
SUGGESTION
Resolved Issues
Incremental Review (commit b936e93)Files changed since last review:
No new issues introduced. Files Reviewed (32+ files)Backend (Rust):
Frontend (TypeScript/React):
Fix these issues in Kilo Cloud Reviewed by kimi-k2.5-0127 · 914,102 tokens |
include driver legend, translations, selection, and tests
1 participant
Summary
This PR introduces a full visual SQL
EXPLAINworkflow across the editor and notebook experience.Users can now open a dedicated Visual Explain modal for supported queries, inspect the execution plan as a graph or table, review the raw driver output, and optionally ask the configured AI provider for a plan analysis.
What Changed
Frontend
VisualExplainModalwith:ExplainGraph,ExplainTableView,ExplainSummaryBar, andExplainAiAnalysiscomponentsBackend / Tauri
explain_query_plancommand to execute and return a structured explain plananalyze_ai_explain_plancommand to send the SQL query plus explain output to the configured AI providerDriver Support
ANALYZEand buffer dataEXPLAIN QUERY PLANoutput through the shared explain-plan modelEXPLAIN ANALYZEusage when supportedQuery Safety / UX
EXPLAINonly runs for supported statement typesANALYZEmay execute data-modifying statementsWhy
The existing SQL tooling could explain queries with AI, but it did not provide a native, structured way to inspect execution plans inside the app. This change closes that gap by combining:
Testing
pnpm test --run tests/utils/explainPlan.test.ts tests/utils/sql.test.tspnpm typecheckpnpm test:rustResults
2files passed,66tests passed317passed,0failedNotes