feat: support EXPLAIN ANALYZE on SQL Server for actual execution plans#361
feat: support EXPLAIN ANALYZE on SQL Server for actual execution plans#361DANze11 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds SQL Server support for PostgreSQL-style EXPLAIN ANALYZE by translating it to SET STATISTICS XML, enabling actual execution plans (with real row counts) while preserving read-only safety via rollback-on-complete execution when options.readonly is enabled.
Changes:
- Extend SQL Server
executeSQL()EXPLAIN handling to parseEXPLAINmodifiers and routeEXPLAIN ANALYZEto a newSET STATISTICS XMLexecution path. - Implement plan extraction from
STATISTICS XMLresults by scanning the final recordsets for aShowPlanXMLpayload. - Add SQL Server integration tests covering
EXPLAIN ANALYZEsemantics, readonly rollback behavior, supported/unsupported option forms, and malformed inputs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/connectors/sqlserver/index.ts | Adds EXPLAIN option parsing and EXPLAIN ANALYZE execution-plan support via SET STATISTICS XML, including readonly rollback handling and plan extraction. |
| src/connectors/tests/sqlserver.integration.test.ts | Adds SQL Server integration coverage for actual-plan generation, execution vs. compile-only behavior, and option/error handling. |
4935a7b to
be92397
Compare
| if (/^explain\b/i.test(afterNoise)) { | ||
| return this.explainQuery(afterNoise.slice("explain".length).trim(), options.readonly); | ||
| const { analyze, query } = SQLServerConnector.parseExplainPrefix( | ||
| afterNoise.slice("explain".length).trim() | ||
| ); | ||
| return analyze | ||
| ? this.explainAnalyzeQuery(query, options) | ||
| : this.explainQuery(query, options.readonly); | ||
| } |
There was a problem hiding this comment.
Wired through rather than rejected — both EXPLAIN paths now bind the caller's parameters.
One correction to the diagnosis: this isn't silent. An undeclared @p1 reaches the server and fails there with Must declare the scalar variable "@p1", so it was a confusing error rather than a wrong result. Worth noting the root is pre-existing on main — explainQuery already dropped parameters before this PR — so the branch only extended the same gap to EXPLAIN ANALYZE. Fixing both together since a caller can't tell the two paths apart.
The binding logic was already duplicated between executeSQL and executeReadOnly, so rather than add a third copy it moves to a shared bindParameters. That works for batch as well as query: node-mssql prepends the matching DECLARE/SET when the request is a batch. Under SHOWPLAN those compile without executing, so a plain EXPLAIN returns the estimated plan for a parameterized query; under STATISTICS XML they execute and the values are real — verified with ActualRows="2" for @p1 = 2 against a three-row source.
| } catch (error) { | ||
| throw new Error(`Failed to explain query: ${(error as Error).message}`); | ||
| } finally { |
There was a problem hiding this comment.
Fixed — it now throws Failed to explain analyze query: ..., with a comment noting why the two are named apart: this path ran the statement, so a failure here can mean the statement itself failed mid-execution.
A plain EXPLAIN maps to SET SHOWPLAN_XML, which compiles the statement without executing it — so its plan carries estimates only. There was no way to get a plan with real row counts. EXPLAIN ANALYZE now maps to SET STATISTICS XML, which runs the statement and returns a plan carrying ActualRows/ActualExecutions, matching the PostgreSQL contract the connector already emulates for EXPLAIN. This also fixes an existing gap: the read-only classifier already recognises EXPLAIN ANALYZE (allowed-keywords.ts) and correctly refuses it for DML, but the SQL Server connector had no handling for it. Both `EXPLAIN ANALYZE <query>` and `EXPLAIN (ANALYZE) <query>` fell through to explainQuery() and reached the server as `ANALYZE <query>` / `(ANALYZE) <query>` — a syntax error. Details: 1. Plan extraction reads the last recordset, not `recordset` (singular). STATISTICS XML interleaves each statement's plan with that statement's own result sets, so the plan sits at no fixed index and the singular accessor would hand back the query's data instead. 2. Option parsing covers the forms the classifier recognises: ANALYZE, ANALYZE VERBOSE, (ANALYZE), (ANALYZE, ...), (ANALYZE false). PostgreSQL-only options (BUFFERS, VERBOSE, COSTS, ...) have no SQL Server counterpart and are refused by name — silently dropping an option would quietly return something other than what was asked for. A disabled ANALYZE (false/off/0) falls back to the estimated plan. The classifier accepts an equals sign there (`ANALYZE = false`) and reads the value after a bare ANALYZE too, so both spellings are honoured here. Otherwise the layers disagree in the dangerous direction: the classifier waives its DML check for what it believes is a non-executing statement, while this connector routes to the path that executes. 3. Unlike EXPLAIN, EXPLAIN ANALYZE executes, so it is not inherently read-only. Under options.readonly the statement runs inside a transaction that always rolls back, guarded by assertNoReadOnlyEscapes with transactionControl set — the rollback is a real transaction a COMMIT could close, which is not true of explainQuery. Sharing that helper also blocks the pass-through data sources it covers: OPENQUERY and friends execute on the remote source, so a local rollback never reaches them, and this path executes by design. 4. The SET STATISTICS XML toggle runs on the same short-lived single-connection pool explainQuery() already uses, so the session state cannot leak onto a shared pool connection where a concurrent query would inherit it. 5. Both EXPLAIN paths now bind the caller's parameters, which the translation previously dropped. Custom tools pass them (custom-tool-handler.ts), so `EXPLAIN SELECT ... WHERE id = @p1` used to reach the server with @p1 undeclared and fail there. The binding logic was already duplicated between executeSQL and executeReadOnly; rather than add a third copy it moves to a shared bindParameters, which works for batch as well as query since node-mssql prepends the matching DECLARE/SET. Under SHOWPLAN those compile without executing, so a plain EXPLAIN returns the estimated plan for a parameterized query; under STATISTICS XML they execute and the values are real. Adds 19 integration tests against a real SQL Server container. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
be92397 to
52ae214
Compare
Problem
On SQL Server,
EXPLAINmaps toSET SHOWPLAN_XML, which compiles a statementwithout executing it. The resulting plan carries estimates only — there is no way
to get a plan with real row counts, which is what you need when the estimates are
the problem.
There is also an existing gap. The read-only classifier already recognises
EXPLAIN ANALYZE(it is a PostgreSQL form, seeutils/allowed-keywords.ts) andcorrectly refuses it for DML, but the SQL Server connector has no handling for it.
Both spellings currently break:
Both are syntax errors.
Solution
EXPLAIN ANALYZEmaps toSET STATISTICS XML, which executes the statement andreturns a plan carrying
ActualRows/ActualExecutions— the PostgreSQLEXPLAIN ANALYZEcontract this connector already emulates for plainEXPLAIN.Notes on the implementation
Plan extraction reads the last recordset, not
recordset(singular).STATISTICS XMLinterleaves each statement's plan with that statement's ownresult sets, so the plan sits at no fixed index — the singular accessor would hand
back the query's data instead of the plan.
Option parsing covers the forms the classifier already recognises:
ANALYZE,ANALYZE VERBOSE,(ANALYZE),(ANALYZE, ...),(ANALYZE false).PostgreSQL-only options (
BUFFERS,VERBOSE,COSTS, …) have no SQL Servercounterpart and are refused by name rather than silently dropped — quietly
ignoring an option would return something other than what was asked for. A
disabled
ANALYZE(false/off/0) falls back to the estimated plan, mirroringthe classifier's own negative lookahead.
Read-only is handled explicitly. Unlike
EXPLAIN,EXPLAIN ANALYZEexecutes,so it is not inherently read-only the way
explainQuery()is. Underoptions.readonlythe statement runs inside a transaction that always rolls back,reusing the reasoning and keyword guards of
executeReadOnly()(#342, #350).Session isolation is preserved. The
SET STATISTICS XMLtoggle runs on thesame short-lived single-connection pool
explainQuery()already uses, so thesession state cannot leak onto a shared pool connection where a concurrent query
would inherit it.
Tests
12 new integration tests against a real SQL Server container, covering: actual
plan contents (
RunTimeInformation/ActualRows), that the statement reallyexecutes (unlike
EXPLAIN), rollback under readonly, the parenthesized forms,disabled
ANALYZEfalling back to an estimated plan, refusal of PostgreSQL-onlyoptions, and the malformed cases.
The full
sqlserver.integration.test.tssuite passes (50 pre-existing + 12 new).No changes to other connectors;
pnpm test:unitis unchanged.