fix(plugins): run SQL Server imports as GO batches and end each statement of a SQL Server dump with GO - #3122
Merged
Merged
Conversation
…ment of a SQL Server dump with GO
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
This branch was successfully deployed
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.
What was wrong
File > Import of a
.sqlfile into a SQL Server connection split the file at every;and sent each piece on its own throughexecute(query:).SQLFileParserknew the.slashLineTerminatorsfact (Oracle) but not.batchSeparatorLines, theGOfact #3105 gave the SQL Server grammar, so aGOline 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: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 theDROP, thenGO\nCREATE PROCEDURE dbo.p AS SET NOCOUNT ONfailed. The procedure was gone afterwards.;was one piece holding everyGOline: Msg 102 "Incorrect syntax near 'GO'".DECLARE @x INT = 1;thenINSERT ... 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 noGOline, 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
.batchSeparatorLines,SQLFileParserkeeps the text of each batch verbatim (its;and its comments), ends the batch at aGOline, repeats it forGO n, and never hands theGOline on. TheGOrule is not re-implemented:SQLBatchSeparatorgets a publicline(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 aGOline 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.;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 withoutGOlines imports a piece at a time instead of being read whole.;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.shsends its import route throughexecuteBatchand still passes, 26 texts, 0 failures.executeBatch.PluginImportDataSinkgainsexecute(statement:line:)(additive, default callsexecute(statement:)), and the SQL import plugin passes each statement's line.ImportDataSinkAdaptersends a SQL Server batch whole throughexecuteBatchwhen the driver declaresresultSetBatches(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 throughBatchErrorText: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 nis honoured and counted. The parser hands a repeated batch out once per run, lazily, andcountStatementsadds the runs up rather than walking them, soGO 2147483647does not hang the dialog's count.GOline, the way SQL Server Management Studio writes a script.PluginExportDataSourcegainslexicalFeatures(additive, default empty);ExportDataSourceAdapterreports the engine's grammar, and the SQL export plugin ends each statement (DDL, drops, comments, indexes, grants,INSERTs and theSET IDENTITY_INSERTpair) with\nGO\nwhen the engine readsGOlines. Every other engine's dump is byte-for-byte what it was.Built and tested
verify.sh build: PASS.verify.sh plugins(AllPlugins): PASS.xcodebuild build-for-testingon 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 overSQLFileParserBatchTests,SQLServerImportBatchTests,SQLFileParserTests,SQLFileParserPLSQLTests,SQLScriptTextTests,SqlFileImportSourceCleanupTests,ImportDataSinkAdapterMappingTests,SQLImportTransactionTestsandSQLImportFailureTests. Since 21:33 every XCTest run on this machine, in every worktree, times out with "Timed out initiating control session with daemon" (a wedgedtestmanagerd), so the run after the rebase, andSQLExportBatchSeparatorTestswith 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 aswift buildharness outside XCTest: all pass. The same harness turnsA GO after a literal that closes on its line is codered 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 andDECLAREscripts;GO nand its count, up toInt32.max; accepted and rejectedGOspellings;GOinside literals, quoted identifiers and comments; comments and line numbers kept; empty batches skipped; CRLF and CR; a chunk boundary at every offset around aGOline 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 withoutresultSetBatches, leaves other engines alone; the whole pipeline (SqlFileImportSource+SQLImportPlugin+ImportDataSinkAdapteroverPluginDriverAdapter) sends noGOand reportsline 3withLine 5: ....SQLExportBatchSeparatorTests(new): the adapter reports SQL Server'sGOlines 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 noGOline.SQLBatchSeparatorTests(package, 23 pass): the new entry point agrees with the scanner line by line.scripts/check-mssql-sql-import.sh(new, committed) builds a harness from the realSQLFileParser, the real MSSQL plugin sources andLibs/libsybdb.a, imports each script as the app does, and reads the database back. 24/24 pass: the Compare script leavesdbo.import_pdefined asCREATE PROCEDURE dbo.import_p AS SET NOCOUNT ON; SELECT 1;and runnable, the SSMS script inserts both rows,@xarrives,GO 3inserts three rows, a routine keeps its comments, a literal keeps itsGOline, an error at file line 6 is reported as line 6, a dump withGOlines 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 andexecute(query:)fail with Msg 2812, Msg 102 and Msg 137 and drop the procedure.swift test --filter TableProSQLGrammarTestslocally: 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 lintover 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.lexicalFeaturesandPluginImportDataSink.execute(statement:line:), both with defaults, so the change is additive and reuses kit 33 (v0.75.0 shipped 32).Not covered
GOline between the two fixes it.INSERTs only, with no view or routine, and are unchanged.Found while fixing #3078 (#3105).