fix(datagrid): show typed EXPLAIN results in the plan viewer (#1480)#1488
Merged
Conversation
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1480
Problem
Running
EXPLAIN ANALYZE(orEXPLAIN FORMAT=JSON/FORMAT=TREE) by typing it in the editor put the plan into the data grid, where it collapsed to one truncated line (-> Limit: 5 row(s) (cost=… (actual…) and was unreadable.TablePro already has a plan viewer (
ExplainResultViewwith Raw/Tree/Diagram). The toolbar "Explain" button routes into it, but a hand-typed explain statement went through the normal query path into the grid, wheresanitizedForCellDisplayreplaces every newline with a space and truncates at 300 chars. The two paths were split; the typed one never reached the viewer.Fix
Route typed explain results to the existing viewer, reusing all the current explain infrastructure.
QueryClassifier.isExplainStatement(_:)detects a leadingEXPLAINwith a word boundary (coversEXPLAIN,EXPLAIN ANALYZE,EXPLAIN (…),EXPLAIN FORMAT=JSON,EXPLAIN QUERY PLAN; excludesDESCRIBE,DESC,EXPLAINING).applyPhase1Resultroutes to a newapplyExplainResultwhen the statement is an explain and the result has exactly one column. The single-column condition is the safe trigger: it captures the unreadable tree/JSON/text plans while leaving multi-column tabular plans (MySQL's plainEXPLAIN, SQLite'sEXPLAIN QUERY PLAN) in the grid, where a table is the right presentation. This one site covers both the normal and parameterized typed paths.resolveTableEditabilityreturns non-editable for explain statements, so anEXPLAIN … FROM ordersno longer triggers a needless schema/count fetch.Untouched on purpose:
sanitizedForCellDisplay(correct for the grid) and the working toolbar variant path.Behavior
EXPLAIN ANALYZE SELECT …(MySQL) → plan viewer, Raw mode: full multi-line tree, monospaced, scrollable, with Copy.EXPLAIN FORMAT=JSON SELECT …→ plan viewer with Diagram/Tree/Raw (JSON parsed into the tree).EXPLAIN SELECT …(MySQL traditional, multi-column) → stays in the grid as a table. No regression.EXPLAIN …(singleQUERY PLANcolumn) → plan viewer.Tests
QueryClassifierExplainTestscovers the detection (the check that would have caught this class of bug). The behavioral routing needs the coordinator stack, so it is verified manually.How to verify
EXPLAIN ANALYZE SELECT * FROM your_table LIMIT 5;.Follow-up (not in this PR)
MySQL's
EXPLAIN ANALYZEtree text only gets Raw mode; the parser is JSON-only. A tree-text parser would unlock Tree/Diagram for it.