From 99213d8d64e29db4f15e009091fa739e9da39c3c Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 17 Jul 2026 12:34:41 +0700 Subject: [PATCH 1/2] fix(editor): stop treating a trailing comment as a second SQL statement --- CHANGELOG.md | 1 + .../Utilities/SQL/SQLStatementScanner.swift | 37 +++++--- .../Execution/ExecutionGateTests.swift | 13 +++ .../Utilities/SQL/QueryClassifierTests.swift | 19 ++++ .../SQLStatementScannerLocatedTests.swift | 14 +++ .../Utilities/SQLStatementScannerTests.swift | 87 ++++++++++++++++++- 6 files changed, 158 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa68ed23c..031c4128a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed a query with a comment after the closing semicolon, such as `SELECT 1; -- note`, being run as two statements, with the trailing comment sent to the server as a failing second statement. Running a comment-only query or selection now does nothing instead of producing a server error, and AI and MCP clients are no longer told such a query is multi-statement. (#1895) - Fixed duplicating a connection dropping its Cloudflare Tunnel and Cloud SQL Auth Proxy settings and stored secrets. - Fixed the tunnel panes warning about only some of the other enabled connection methods. Each pane now lists every conflicting method with a button to turn it off. - Fixed Copy as, the context menu's Delete, and the Edit menu's copy and delete commands acting on only the active row instead of every row in a cell-range or column selection in the data grid. (#1898) diff --git a/TablePro/Core/Utilities/SQL/SQLStatementScanner.swift b/TablePro/Core/Utilities/SQL/SQLStatementScanner.swift index 95a4aeeee..bd0afc8b3 100644 --- a/TablePro/Core/Utilities/SQL/SQLStatementScanner.swift +++ b/TablePro/Core/Utilities/SQL/SQLStatementScanner.swift @@ -15,7 +15,8 @@ enum SQLStatementScanner { /// Returns statements with trailing semicolons stripped, for driver execution. static func allStatements(in sql: String, dialect: SqlDialect = .generic) -> [String] { var results: [String] = [] - scan(sql: sql, cursorPosition: nil, dialect: dialect) { rawSQL, _ in + scan(sql: sql, cursorPosition: nil, dialect: dialect) { rawSQL, _, hasStatementContent in + guard hasStatementContent else { return true } var trimmed = rawSQL.trimmingCharacters(in: .whitespacesAndNewlines) if trimmed.hasSuffix(";") { trimmed = String(trimmed.dropLast()) @@ -32,7 +33,8 @@ enum SQLStatementScanner { /// Returns statements preserving trailing semicolons, for display/history/favorites. static func allStatementsPreservingSemicolons(in sql: String) -> [String] { var results: [String] = [] - scan(sql: sql, cursorPosition: nil) { rawSQL, _ in + scan(sql: sql, cursorPosition: nil) { rawSQL, _, hasStatementContent in + guard hasStatementContent else { return true } let trimmed = rawSQL.trimmingCharacters(in: .whitespacesAndNewlines) let withoutSemicolon = trimmed.hasSuffix(";") ? String(trimmed.dropLast()).trimmingCharacters(in: .whitespacesAndNewlines) @@ -58,7 +60,7 @@ enum SQLStatementScanner { static func locatedStatementAtCursor(in sql: String, cursorPosition: Int, dialect: SqlDialect = .generic) -> LocatedStatement { var result = LocatedStatement(sql: "", offset: 0) - scan(sql: sql, cursorPosition: cursorPosition, dialect: dialect) { rawSQL, offset in + scan(sql: sql, cursorPosition: cursorPosition, dialect: dialect) { rawSQL, offset, _ in result = LocatedStatement(sql: rawSQL, offset: offset) return false } @@ -77,22 +79,25 @@ enum SQLStatementScanner { private static let newline = UInt16(UnicodeScalar("\n").value) private static let backslash = UInt16(UnicodeScalar("\\").value) private static let dollar = UInt16(UnicodeScalar("$").value) + private static let exclamationMark = UInt16(UnicodeScalar("!").value) + private static let space = UInt16(UnicodeScalar(" ").value) + private static let tab = UInt16(UnicodeScalar("\t").value) + private static let carriageReturn = UInt16(UnicodeScalar("\r").value) + + private static func isWhitespace(_ ch: UInt16) -> Bool { + ch == space || ch == tab || ch == newline || ch == carriageReturn + } private static func scan( sql: String, cursorPosition: Int?, dialect: SqlDialect = .generic, - onStatement: (_ rawSQL: String, _ offset: Int) -> Bool + onStatement: (_ rawSQL: String, _ offset: Int, _ hasStatementContent: Bool) -> Bool ) { let nsQuery = sql as NSString let length = nsQuery.length guard length > 0 else { return } - guard nsQuery.range(of: ";").location != NSNotFound else { - _ = onStatement(sql, 0) - return - } - let safePosition = cursorPosition.map { min(max(0, $0), length) } var currentStart = 0 @@ -102,6 +107,7 @@ enum SQLStatementScanner { var inBlockComment = false var inDollarQuote = false var dollarTag = "" + var hasStatementContent = false let dollarQuotesEnabled = dialect.supportsDollarQuotes var i = 0 @@ -143,6 +149,9 @@ enum SQLStatementScanner { } if !inString && ch == slash && i + 1 < length && nsQuery.character(at: i + 1) == star { + if i + 2 < length && nsQuery.character(at: i + 2) == exclamationMark { + hasStatementContent = true + } inBlockComment = true i += 2 continue @@ -170,6 +179,7 @@ enum SQLStatementScanner { case .opener(let openerLength, let tag) = SqlDollarQuote.scanOpener(at: i, in: nsQuery, bufLen: length) { inDollarQuote = true dollarTag = tag + hasStatementContent = true i += openerLength continue } @@ -180,15 +190,18 @@ enum SQLStatementScanner { if let cursor = safePosition { if cursor >= currentStart && cursor <= stmtEnd { let stmtRange = NSRange(location: currentStart, length: stmtEnd - currentStart) - _ = onStatement(nsQuery.substring(with: stmtRange), currentStart) + _ = onStatement(nsQuery.substring(with: stmtRange), currentStart, hasStatementContent) return } } else { let stmtRange = NSRange(location: currentStart, length: stmtEnd - currentStart) - if !onStatement(nsQuery.substring(with: stmtRange), currentStart) { return } + if !onStatement(nsQuery.substring(with: stmtRange), currentStart, hasStatementContent) { return } } currentStart = stmtEnd + hasStatementContent = false + } else if !isWhitespace(ch) { + hasStatementContent = true } i += 1 @@ -196,7 +209,7 @@ enum SQLStatementScanner { if currentStart < length { let stmtRange = NSRange(location: currentStart, length: length - currentStart) - _ = onStatement(nsQuery.substring(with: stmtRange), currentStart) + _ = onStatement(nsQuery.substring(with: stmtRange), currentStart, hasStatementContent) } } } diff --git a/TableProTests/Core/Services/Execution/ExecutionGateTests.swift b/TableProTests/Core/Services/Execution/ExecutionGateTests.swift index 4d93ba2a7..470600502 100644 --- a/TableProTests/Core/Services/Execution/ExecutionGateTests.swift +++ b/TableProTests/Core/Services/Execution/ExecutionGateTests.swift @@ -375,6 +375,19 @@ struct ExecutionGateTests { #expect(allowed.isAuthorized) } + @Test("A trailing comment after the semicolon is not denied as multi-statement") + func trailingCommentNotDeniedAsMultiStatement() async { + let confirm = StubConfirming(answer: true) + let auth = StubAuthenticating(answer: true) + let gate = makeGate(level: .silent, confirm: confirm, auth: auth) + + let decision = await gate.authorize( + makeRequest(sql: "SELECT 1; -- note", kind: .readQuery, capabilities: [.mayWrite]) + ) + + #expect(decision.isAuthorized) + } + // MARK: - Pre-cleared and cannot-prompt @Test("Pre-cleared caller skips confirmation and auth") diff --git a/TableProTests/Core/Utilities/SQL/QueryClassifierTests.swift b/TableProTests/Core/Utilities/SQL/QueryClassifierTests.swift index 1eca6dd73..874f8f5ad 100644 --- a/TableProTests/Core/Utilities/SQL/QueryClassifierTests.swift +++ b/TableProTests/Core/Utilities/SQL/QueryClassifierTests.swift @@ -93,3 +93,22 @@ struct QueryClassifierKeywordBoundaryTests { #expect(QueryClassifier.classifyTier("UPDATE\nt SET x = 1", databaseType: .mysql) == .write) } } + +@Suite("QueryClassifier isMultiStatement") +struct QueryClassifierMultiStatementTests { + @Test("A trailing comment after the terminating semicolon is not a second statement") + func trailingCommentIsNotMultiStatement() { + #expect(!QueryClassifier.isMultiStatement("SELECT 1; -- note", databaseType: .mysql)) + #expect(!QueryClassifier.isMultiStatement("SELECT 1; /* note */", databaseType: .postgresql)) + } + + @Test("Two real statements are still multi-statement") + func twoRealStatementsAreMultiStatement() { + #expect(QueryClassifier.isMultiStatement("SELECT 1; SELECT 2", databaseType: .mysql)) + } + + @Test("A comment-only query is not multi-statement") + func commentOnlyQueryIsNotMultiStatement() { + #expect(!QueryClassifier.isMultiStatement("-- note", databaseType: .mysql)) + } +} diff --git a/TableProTests/Core/Utilities/SQLStatementScannerLocatedTests.swift b/TableProTests/Core/Utilities/SQLStatementScannerLocatedTests.swift index a45a7b1f9..431746db1 100644 --- a/TableProTests/Core/Utilities/SQLStatementScannerLocatedTests.swift +++ b/TableProTests/Core/Utilities/SQLStatementScannerLocatedTests.swift @@ -180,4 +180,18 @@ struct SQLStatementScannerLocatedTests { #expect(first.sql == "SELECT 'it''s here';") #expect(first.offset == 0) } + + @Test("Cursor inside a trailing comment still returns the raw comment text") + func cursorInsideTrailingCommentReturnsRawText() { + let sql = "SELECT 1; -- note" + let located = SQLStatementScanner.locatedStatementAtCursor(in: sql, cursorPosition: 15) + #expect(located.sql == " -- note") + #expect(located.offset == 9) + } + + @Test("statementAtCursor returns comment text for a cursor sitting in a trailing comment") + func statementAtCursorReturnsCommentTextForCursorInComment() { + let result = SQLStatementScanner.statementAtCursor(in: "SELECT 1; -- note", cursorPosition: 15) + #expect(result == "-- note") + } } diff --git a/TableProTests/Core/Utilities/SQLStatementScannerTests.swift b/TableProTests/Core/Utilities/SQLStatementScannerTests.swift index f93769b36..30bcc6d5e 100644 --- a/TableProTests/Core/Utilities/SQLStatementScannerTests.swift +++ b/TableProTests/Core/Utilities/SQLStatementScannerTests.swift @@ -109,6 +109,91 @@ final class SQLStatementScannerTests: XCTestCase { ) } + // MARK: - Comment-Only Segments + + func testTrailingLineCommentAfterSemicolonIsDropped() { + XCTAssertEqual( + SQLStatementScanner.allStatements(in: "SELECT 1; -- note"), + ["SELECT 1"] + ) + } + + func testTrailingBlockCommentAfterSemicolonIsDropped() { + XCTAssertEqual( + SQLStatementScanner.allStatements(in: "SELECT 1; /* note */"), + ["SELECT 1"] + ) + } + + func testMiddleCommentOnlySegmentIsDropped() { + XCTAssertEqual( + SQLStatementScanner.allStatements(in: "SELECT 1; /* x */; SELECT 2"), + ["SELECT 1", "SELECT 2"] + ) + } + + func testCommentOnlyWholeInputReturnsNoStatements() { + XCTAssertEqual(SQLStatementScanner.allStatements(in: "-- note"), []) + XCTAssertEqual(SQLStatementScanner.allStatements(in: "/* note */"), []) + } + + func testMixedCommentAndWhitespaceMultilineSegmentIsDropped() { + let sql = "SELECT 1;\n -- first\n /* second */ \n; SELECT 2" + XCTAssertEqual( + SQLStatementScanner.allStatements(in: sql), + ["SELECT 1", "SELECT 2"] + ) + } + + func testConsecutiveSemicolonsStillDropped() { + XCTAssertEqual( + SQLStatementScanner.allStatements(in: "SELECT 1;;"), + ["SELECT 1"] + ) + } + + func testUnclosedTrailingBlockCommentIsDropped() { + XCTAssertEqual( + SQLStatementScanner.allStatements(in: "SELECT 1; /* note"), + ["SELECT 1"] + ) + } + + func testLineCommentAtEndOfInputIsDropped() { + XCTAssertEqual( + SQLStatementScanner.allStatements(in: "SELECT 1; --"), + ["SELECT 1"] + ) + } + + func testMySQLVersionedCommentSegmentIsKept() { + XCTAssertEqual( + SQLStatementScanner.allStatements(in: "/*!40101 SET @OLD=1 */; SELECT 1", dialect: .mysql), + ["/*!40101 SET @OLD=1 */", "SELECT 1"] + ) + } + + func testVersionedCommentKeptRegardlessOfDialect() { + XCTAssertEqual( + SQLStatementScanner.allStatements(in: "/*!40101 SET @OLD=1 */;", dialect: .generic), + ["/*!40101 SET @OLD=1 */"] + ) + } + + func testInStatementCommentsArePreserved() { + XCTAssertEqual( + SQLStatementScanner.allStatements(in: "SELECT 1 -- hint\n; SELECT 2 /* keep */"), + ["SELECT 1 -- hint", "SELECT 2 /* keep */"] + ) + } + + func testPreservingSemicolonsDropsCommentOnlySegment() { + XCTAssertEqual( + SQLStatementScanner.allStatementsPreservingSemicolons(in: "SELECT 1; -- note"), + ["SELECT 1;"] + ) + } + // MARK: - Dollar Quoting (PostgreSQL) func testDollarQuotedDoBlockKeepsInternalSemicolons() { @@ -238,7 +323,7 @@ final class SQLStatementScannerTests: XCTestCase { ) } - func testNoSemicolonsFastPath() { + func testNoSemicolonsWholeInputIsOneStatement() { let sql = "SELECT * FROM users" XCTAssertEqual( SQLStatementScanner.statementAtCursor(in: sql, cursorPosition: 5), From fd97e1aaf4c245fecf3c0ea9fba607437e77253c Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 17 Jul 2026 12:34:42 +0700 Subject: [PATCH 2/2] ci: run the SQL statement scanner cursor tests on macOS CI again --- .github/macos-test-quarantine.txt | 1 - .../Core/Utilities/SQLStatementScannerLocatedTests.swift | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/macos-test-quarantine.txt b/.github/macos-test-quarantine.txt index 2c9670781..c4e1d9c38 100644 --- a/.github/macos-test-quarantine.txt +++ b/.github/macos-test-quarantine.txt @@ -57,4 +57,3 @@ DataChangeManagerTests CoordinatorEditorLoadTests CommandActionsDispatchTests ChatToolSpecCopilotTests -SQLStatementScannerLocatedTests diff --git a/TableProTests/Core/Utilities/SQLStatementScannerLocatedTests.swift b/TableProTests/Core/Utilities/SQLStatementScannerLocatedTests.swift index 431746db1..016738ec1 100644 --- a/TableProTests/Core/Utilities/SQLStatementScannerLocatedTests.swift +++ b/TableProTests/Core/Utilities/SQLStatementScannerLocatedTests.swift @@ -143,7 +143,7 @@ struct SQLStatementScannerLocatedTests { @Test("Handles very large input without crashing") func largeInput() { var parts: [String] = [] - for i in 0..<200 { + for i in 0..<300 { parts.append("SELECT \(i) FROM very_long_table_name_for_testing;") } let sql = parts.joined(separator: " ")