Skip to content

fix(plugin-mssql): run SQL Server scripts as GO batches and show every result set they return - #3105

Merged
datlechin merged 2 commits into
mainfrom
fix/mssql-batches
Sep 24, 2026
Merged

datlechin merged 2 commits into
mainfrom
fix/mssql-batches

Conversation

@datlechin

Copy link
Copy Markdown
Member

Fixes #3078.

Root cause

The editor split a SQL Server script at every ; and sent each piece in its own request. SQL Server treats every request as a batch, and a local variable, a table variable, a TRY...CATCH or an IF ... BEGIN ... END block lives only inside the batch that declares it. So DECLARE @sn was gone by the time the next SELECT ran: Statement 2/5 failed: Must declare the scalar variable "@sn".

The editor could not send a script whole because the driver could only hand back one result set per request. The FreeTDS loop put every later result set's rows under the first one's columns, and the streaming path trapped when a later result set was wider.

What changed

Scripts now run the way sqlcmd, SSMS and Azure Data Studio run them: a batch at a time, cut at GO lines, with every result set a batch returns in its own result tab.

  • GO is a statement boundary in the SQL Server grammar. It is a new grammar fact, batchSeparatorLines, with its PluginKit partner. The gutter, run at cursor, navigation, Explain and the Safe Mode gate all see it, so a DROP behind a GO line is classified, and GO never reaches the server. GO n runs the batch n times.
  • PluginKit executeBatch(query:rowCap:parameters:) -> PluginBatchResult? returns every complete result set, the rows affected, and each server error with its line and procedure. It is additive, with a default of nil and a resultSetBatches capability, and it reuses kit 33, already pending this cycle. The ABI check shows 22 additions and 0 removals. An installed MSSQL plugin that predates this keeps running statement by statement, and refuses GO n with a message rather than running the batch once.
  • One FreeTDS result reader for every call:
    • Each result set keeps its own columns.
    • A FAIL from dbresults fails only that statement.
    • Every exit reads to the end, skips unread rows or leaves them for the next call, so the connection can no longer lock up with db-lib 20019.
    • Server errors raised after a result set's columns (SELECT 1/0, a conversion error mid-scan) are reported instead of showing an empty or short result.
    • -1 counts are ignored.
    • A severity 20 error is a lost connection.
    • PRINT reaches the Output view through fetchServerOutput.
  • Parameterized batches bind in their own scope. :name values are bound with a leading DECLARE on the batch's first line, so BEGIN TRAN, #temp tables and USE behave as they would unbound. sp_executesql is kept only for batches that must be alone: routine definitions and bare procedure calls.
  • The app's batch path. It lives in QueryExecutionCoordinator+Batches.
    • SQL Server batch runs open no app transaction, as in SSMS. The script's own BEGIN TRAN and TRY...CATCH decide, and a transaction the script leaves open is reported.
    • Errors read Batch 2/3 failed: Line 7: …, with the line mapped to the editor.
    • A batch of plain queries and DECLARE/SET names each result after its query and keeps Load More, unless the query reads a variable. Other batches number their results.
    • A result that cannot be run again on its own is sorted in place, never by re-sending the editor text.
  • Which path runs what. A lone plain query keeps the single-statement path, with paging and editing. Any other lone statement, such as EXEC sp_help, runs as a batch and shows all its result sets.

Before / After

The reporter's script on SQL Server (Azure SQL Edge in Docker). Before: TablePro 0.75.0 with the published MSSQL plugin 1.0.46:

Before: Statement 2/5 failed: Must declare the scalar variable "@sn"

After: this branch. One batch, four result sets (serialnew, v_wms_joined, drm_report_n, serial_existed), the last one shown:

After: Result 4 of 4 shows the serial_existed row

A second batch after a GO that hits a missing table. The line is the editor line, and the PRINT from that batch is shown before the error. The grid under the banner shows the previous run's columns. That behaviour is already on main for every engine: it reproduces on SQLite through the unchanged statement-by-statement path. It is not part of this change.

After: Batch 2/2 failed: Line 5: Invalid object name 'missing_table'

Verification

  • verify.sh build: PASS. verify.sh plugins (AllPlugins, including the registry-only MSSQL driver): PASS.
  • verify.sh test over 29 suites that own the changed types, including the planner, sort, batch run, failure report, gate, status model, adapter, SQLScriptText, scanner and all MSSQL suites: 531/531.
  • Package tests. TableProSQLGrammarTests: 51, including 22 new GO separator tests. TableProQueryTests: 60. TableProMSSQLCoreTests: 78 swift-testing and 66 XCTest.
  • scripts/check-mssql-batch-results.sh is a new live check. It builds the real MSSQL plugin sources and runs them against Azure SQL Edge in Docker: 40/40. Against main's driver sources the checks fail, 7 of the first 12, and the stream path then traps with index out of range.
  • The PluginKit ABI check against the merge base is additive only. The kit is already at 33 this cycle, so there is no further bump.
  • Tested end to end in a sandboxed Debug build with the branch plugin, before the review rounds: the reporter's script and the GO error case above. In-place sorting and its reset are covered by MainContentCoordinatorSortTests.
  • No UI automation: the UI test environment has no SQL Server, and every behaviour here depends on the server. The live check covers the driver end to end.
  • Review: Codex was unavailable (usage limit until Sep 29), so the diff went through two rounds of a multi-agent review workflow, with each finding checked by a separate skeptic. 22 confirmed findings were fixed, one is a known limitation listed below, and the rest were refuted or predate this branch. A separate security review found nothing this branch introduces.

Release note

MSSQL is a registry-only plugin, so the fix reaches users when this app release ships and the MSSQL plugin is re-published against kit 33. An older installed MSSQL plugin keeps today's behaviour, minus GO being sent to the server.

Known limitations

  • db-lib discards which statement produced a count, so a DECLARE @x int = 1 or SET @x = … adds 1 to a batch's rows affected. main shows the same count for those statements.
  • The driver's ? rewriter still differs from the app's :name scanner inside bracketed identifiers such as [Total:USD]. That difference is already on main.

@mintlify

mintlify Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 24, 2026, 8:04 AM

💡 Tip: Enable Automations to automatically generate PRs for you.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
@datlechin
datlechin merged commit de028a0 into main Sep 24, 2026
4 checks passed
@datlechin
datlechin deleted the fix/mssql-batches branch September 24, 2026 08:05
This was referenced Sep 24, 2026

This branch was successfully deployed

1 active deployment
staging - docs — 4123343a Deployed Sep 24, 2026 by mintlify[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SQL Server batch variables are lost between statements

1 participant