Skip to content

fix(plugins): run SQL Server imports as GO batches and end each statement of a SQL Server dump with GO - #3122

Merged
datlechin merged 1 commit into
mainfrom
fix/sql-import-go-batches
Sep 24, 2026
Merged

datlechin merged 1 commit into
mainfrom
fix/sql-import-go-batches

Conversation

@datlechin

@datlechin datlechin commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

What was wrong

File > Import of a .sql file into a SQL Server connection split the file at every ; and sent each piece on its own through execute(query:). SQLFileParser knew the .slashLineTerminators fact (Oracle) but not .batchSeparatorLines, the GO fact #3105 gave the SQL Server grammar, so a GO line was just text glued to the next statement. Measured on main against Azure SQL Edge 15 with the real parser and the real MSSQL driver:

  • A Compare-style script (DROP PROCEDURE [dbo].[p]; / GO / CREATE PROCEDURE dbo.p AS SET NOCOUNT ON; SELECT 1; / GO) became four pieces. GO\nDROP PROCEDURE ... answered Msg 2812 "Could not find stored procedure 'GO'" and still ran the DROP, then GO\nCREATE PROCEDURE dbo.p AS SET NOCOUNT ON failed. The procedure was gone afterwards.
  • An SSMS script with no ; was one piece holding every GO line: Msg 102 "Incorrect syntax near 'GO'".
  • DECLARE @x INT = 1; then INSERT ... VALUES (@x); ran as two requests: Msg 137 "Must declare the scalar variable", the SQL Server batch variables are lost between statements #3078 bug through a second door.

A second defect sat behind the first. TablePro's own SQL export for SQL Server wrote a ; after each statement and no GO line, so a dump holding a view, routine or trigger was one batch. SQL Server refuses those unless they come first in their batch (Msg 111, measured: the whole batch fails and nothing in it runs). sqlcmd and SSMS already could not run such a dump. Once the import reads SQL Server files the way sqlcmd does, TablePro's own import could not run it either, so the export fix ships here.

What changed

  • Import reads a SQL Server script the way sqlcmd does. On a grammar with .batchSeparatorLines, SQLFileParser keeps the text of each batch verbatim (its ; and its comments), ends the batch at a GO line, repeats it for GO n, and never hands the GO line on. The GO rule is not re-implemented: SQLBatchSeparator gets a public line(startingAt:in:length:grammar:) for a reader that tracked the line start itself, and the editor's scanner goes through the same code. The parser holds a GO line back until the chunk buffer holds the whole line, so a 64 KiB chunk boundary anywhere in or around it changes nothing. Comments stay so the server's line numbers count the file's lines and a routine keeps the comments written in it.
  • Memory stays bounded. A batch that reaches 64 Mi UTF-16 units ends at its next ; outside a literal. SQL Server takes at most 65,536 packets of 4,096 bytes per request, measured as a 133,900,000-unit batch running and a 133,960,000-unit one closing the connection, so the cut only touches batches the server could never take whole, and a large dump written without GO lines imports a piece at a time instead of being read whole.
  • fix(editor): keep a SQL Server MERGE's semicolon after a non-ASCII name, on a table named range, and on SQL import #3120's SQL Server import tests move to batch semantics. They pinned a MERGE's ; surviving a ; split that no longer happens on SQL Server: a batch now arrives as written, every ; kept, so they assert that instead. scripts/check-mssql-merge-terminator.sh sends its import route through executeBatch and still passes, 26 texts, 0 failures.
  • Each batch goes through executeBatch. PluginImportDataSink gains execute(statement:line:) (additive, default calls execute(statement:)), and the SQL import plugin passes each statement's line. ImportDataSinkAdapter sends a SQL Server batch whole through executeBatch when the driver declares resultSetBatches (keeping one row per result set, since an import shows none), and turns the batch's errors into a failure with the file's own line through BatchErrorText: Failed at line 3. Line 5: Invalid object name 'orders'. A driver without batches runs the batch's statements one by one, as the editor does. Every other engine is sent what it was before.
  • GO n is honoured and counted. The parser hands a repeated batch out once per run, lazily, and countStatements adds the runs up rather than walking them, so GO 2147483647 does not hang the dialog's count.
  • SQL Server dumps end every statement with a GO line, the way SQL Server Management Studio writes a script. PluginExportDataSource gains lexicalFeatures (additive, default empty); ExportDataSourceAdapter reports the engine's grammar, and the SQL export plugin ends each statement (DDL, drops, comments, indexes, grants, INSERTs and the SET IDENTITY_INSERT pair) with \nGO\n when the engine reads GO lines. Every other engine's dump is byte-for-byte what it was.
  • PluginKit: both additions are additive; the kit stays at 33, which this release cycle already bumped.

Built and tested

  • verify.sh build: PASS. verify.sh plugins (AllPlugins): PASS. xcodebuild build-for-testing on the final commit: TEST BUILD SUCCEEDED.
  • verify.sh test: before the rebase onto fix(editor): keep a SQL Server MERGE's semicolon after a non-ASCII name, on a table named range, and on SQL import #3120, PASS with 128 cases over SQLFileParserBatchTests, SQLServerImportBatchTests, SQLFileParserTests, SQLFileParserPLSQLTests, SQLScriptTextTests, SqlFileImportSourceCleanupTests, ImportDataSinkAdapterMappingTests, SQLImportTransactionTests and SQLImportFailureTests. Since 21:33 every XCTest run on this machine, in every worktree, times out with "Timed out initiating control session with daemon" (a wedged testmanagerd), so the run after the rebase, and SQLExportBatchSeparatorTests with the export suites, is pending, and this line is updated once it runs. Until then, the parser cases (the new batch cases and fix(editor): keep a SQL Server MERGE's semicolon after a non-ASCII name, on a table named range, and on SQL import #3120's rewritten ones, 24 checks) were run on the final parser in a swift build harness outside XCTest: all pass. The same harness turns A GO after a literal that closes on its line is code red when the line-start flag is read off the first unit of a step instead of the last, at the chunk boundary that falls on the line break inside the literal.
    • SQLFileParserBatchTests (new): Compare, SSMS and DECLARE scripts; GO n and its count, up to Int32.max; accepted and rejected GO spellings; GO inside literals, quoted identifiers and comments; comments and line numbers kept; empty batches skipped; CRLF and CR; a chunk boundary at every offset around a GO line and after a literal that closes on its line; the cut at a ; outside a literal; other engines unchanged.
    • SQLServerImportBatchTests (new): the sink sends a batch whole once, places a batch error on the file's line, falls back to statements without resultSetBatches, leaves other engines alone; the whole pipeline (SqlFileImportSource + SQLImportPlugin + ImportDataSinkAdapter over PluginDriverAdapter) sends no GO and reports line 3 with Line 5: ....
    • SQLExportBatchSeparatorTests (new): the adapter reports SQL Server's GO lines and not MySQL's; a SQL Server dump read back by the SQL Server parser is one statement per batch with the view and the procedure first in theirs; a MySQL dump has no GO line.
    • SQLBatchSeparatorTests (package, 23 pass): the new entry point agrees with the scanner line by line.
  • Live, against Azure SQL Edge 15 in my own database: scripts/check-mssql-sql-import.sh (new, committed) builds a harness from the real SQLFileParser, the real MSSQL plugin sources and Libs/libsybdb.a, imports each script as the app does, and reads the database back. 24/24 pass: the Compare script leaves dbo.import_p defined as CREATE PROCEDURE dbo.import_p AS SET NOCOUNT ON; SELECT 1; and runnable, the SSMS script inserts both rows, @x arrives, GO 3 inserts three rows, a routine keeps its comments, a literal keeps its GO line, an error at file line 6 is reported as line 6, a dump with GO lines creates its view while the same dump without them fails with Msg 111, and a dump cut at semicolons loses no row. The same three scripts through main's parser and execute(query:) fail with Msg 2812, Msg 102 and Msg 137 and drop the procedure.
  • swift test --filter TableProSQLGrammarTests locally: 85 tests pass. On this PR's first CI run, Package Tests was cancelled at its 20-minute limit and Build for testing stalled; main's own run 36026020019 was cancelled the same way at the same time, so those two checks need a rerun rather than a fix. Run iOS Tests passed.
  • verify.sh lint over all 16 changed Swift files: 0 violations. verify.sh docs: PASS. check-ios-shared-isolation.py: clean. verify.sh abi 86aa58744: only added lines, PluginExportDataSource.lexicalFeatures and PluginImportDataSink.execute(statement:line:), both with defaults, so the change is additive and reuses kit 33 (v0.75.0 shipped 32).

Not covered

  • No UI automation: the flow needs a SQL Server, which the UI test runner does not have.
  • The import dialog still labels its count "statements"; on SQL Server it counts batches, which the docs now say.
  • An import into SQL Server now runs each batch whole, as sqlcmd does, so a batch that altered a table and then used the new column in the same batch fails at compile time with Msg 207 (measured) where the old per-statement split ran it. sqlcmd and SSMS fail it the same way; a GO line between the two fixes it.
  • Query result exports as SQL write INSERTs only, with no view or routine, and are unchanged.

Found while fixing #3078 (#3105).

@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, 3:36 PM

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

This branch was successfully deployed

1 active deployment
staging - docs — 2369d284 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.

1 participant