From fb84d82d73545baf647693b6644bc8aaf33d54b0 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 16 Jul 2026 23:22:05 +0700 Subject: [PATCH 1/2] fix(plugin-mysql): report the real error when a query fails while reading rows (#1884) --- CHANGELOG.md | 1 + Plugins/MySQLDriverPlugin/CMariaDB/CMariaDB.h | 1 + .../MariaDBPluginConnection.swift | 65 +++++++++++-------- 3 files changed, 39 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8a28c7db..486502426 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed MySQL and MariaDB queries ending in ORDER BY that could show an empty result with no error. A failure part way through reading rows was treated as a normal end of results, so a real server error looked like a table with no rows. TablePro now shows the error the server reported. (#1884) - Fixed SSH tunnels that could accept a database connection and then go quiet instead of forwarding it or failing, which showed up as MySQL and MariaDB connections timing out while reading the server greeting. A tunnel that cannot open its forwarding channel now gives up after 10 seconds, closes the connection, and logs the reason, instead of leaving the driver to wait out its own timeout with no explanation. (#1883) - Fixed connections being reset when a single database session opened several at once through one SSH tunnel, which could happen while browsing schemas. (#1883) diff --git a/Plugins/MySQLDriverPlugin/CMariaDB/CMariaDB.h b/Plugins/MySQLDriverPlugin/CMariaDB/CMariaDB.h index fed7eb1d5..37995df08 100644 --- a/Plugins/MySQLDriverPlugin/CMariaDB/CMariaDB.h +++ b/Plugins/MySQLDriverPlugin/CMariaDB/CMariaDB.h @@ -10,5 +10,6 @@ #define CMariaDB_h #include "include/mysql.h" +#include "include/mysqld_error.h" #endif /* CMariaDB_h */ diff --git a/Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift b/Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift index fb67df17e..85720d794 100644 --- a/Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift +++ b/Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift @@ -381,7 +381,10 @@ final class MariaDBPluginConnection: @unchecked Sendable { stateLock.unlock() guard let mysql = mysql else { return } - let threadId = mysql_thread_id(mysql) + killQueryOnServer(threadId: mysql_thread_id(mysql)) + } + + private func killQueryOnServer(threadId: UInt) { guard threadId > 0 else { return } let killConn = mysql_init(nil) @@ -389,6 +392,8 @@ final class MariaDBPluginConnection: @unchecked Sendable { var killTimeout: UInt32 = 5 mysql_options(killConn, MYSQL_OPT_CONNECT_TIMEOUT, &killTimeout) + mysql_options(killConn, MYSQL_OPT_READ_TIMEOUT, &killTimeout) + mysql_options(killConn, MYSQL_OPT_WRITE_TIMEOUT, &killTimeout) let killResult = host.withCString { hostPtr in user.withCString { userPtr in @@ -412,6 +417,18 @@ final class MariaDBPluginConnection: @unchecked Sendable { mysql_close(killConn) } + private func consumeCancellation() -> Bool { + stateLock.lock() + defer { stateLock.unlock() } + guard _isCancelled else { return false } + _isCancelled = false + return true + } + + private static func isExpectedInterruption(errno: UInt32, wasTruncated: Bool) -> Bool { + wasTruncated && errno == UInt32(ER_QUERY_INTERRUPTED) + } + // MARK: - Query Execution func executeQuery(_ query: String, rowCap: Int? = nil) async throws -> MariaDBPluginQueryResult { @@ -508,20 +525,8 @@ final class MariaDBPluginConnection: @unchecked Sendable { var truncated = false while let rowPtr = mysql_fetch_row(resultPtr) { - stateLock.lock() - let shouldCancel = _isCancelled - if shouldCancel { _isCancelled = false } - stateLock.unlock() - if shouldCancel { + if consumeCancellation() { while mysql_fetch_row(resultPtr) != nil {} - if mysql_errno(mysql) != 0 { - let errorMsg = String(cString: mysql_error(mysql)) - mysql_free_result(resultPtr) - throw MariaDBPluginError( - code: mysql_errno(mysql), - message: "Error draining result set during cancellation: \(errorMsg)", - sqlState: nil) - } mysql_free_result(resultPtr) throw CancellationError() } @@ -561,15 +566,20 @@ final class MariaDBPluginConnection: @unchecked Sendable { if truncated { logger.warning("Result set truncated at \(maxRows) rows") + killQueryOnServer(threadId: mysql_thread_id(mysql)) while mysql_fetch_row(resultPtr) != nil {} - if mysql_errno(mysql) != 0 { - let errorMsg = String(cString: mysql_error(mysql)) - mysql_free_result(resultPtr) - throw MariaDBPluginError( - code: mysql_errno(mysql), - message: "Error draining result set: \(errorMsg)", - sqlState: nil) - } + } + + if consumeCancellation() { + mysql_free_result(resultPtr) + throw CancellationError() + } + + let fetchErrno = mysql_errno(mysql) + if fetchErrno != 0, !Self.isExpectedInterruption(errno: fetchErrno, wasTruncated: truncated) { + let error = getError() + mysql_free_result(resultPtr) + throw error } mysql_free_result(resultPtr) @@ -704,13 +714,12 @@ final class MariaDBPluginConnection: @unchecked Sendable { while true { let fetchStatus = mysql_stmt_fetch(stmt) - if fetchStatus != 0 && fetchStatus != MYSQL_DATA_TRUNCATED { break } + if fetchStatus == MYSQL_NO_DATA { break } + if fetchStatus != 0, fetchStatus != MYSQL_DATA_TRUNCATED { + throw getStmtError(stmt) + } - stateLock.lock() - let shouldCancel = _isCancelled - if shouldCancel { _isCancelled = false } - stateLock.unlock() - if shouldCancel { + if consumeCancellation() { throw CancellationError() } From a4e7e52580d6099e1d3979439fe45405f71fda5b Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 16 Jul 2026 23:22:26 +0700 Subject: [PATCH 2/2] fix(editor): apply the query result row cap without rewriting the SQL (#1884) --- CHANGELOG.md | 5 + .../PluginDatabaseDriver.swift | 5 - .../QueryExecutionCoordinator+Helpers.swift | 18 +- ...yExecutionCoordinator+MultiStatement.swift | 6 +- ...QueryExecutionCoordinator+Parameters.swift | 14 +- .../QueryExecutionCoordinator+RowLimit.swift | 39 ---- .../Core/Plugins/PluginDriverAdapter.swift | 6 - .../Core/Services/Query/QueryExecutor.swift | 3 +- ...tInjector.swift => SQLLimitDetector.swift} | 148 +++++---------- .../MainContentCoordinator+QueryHelpers.swift | 4 +- .../Views/Main/MainContentCoordinator.swift | 8 +- .../Services/Query/QueryExecutorTests.swift | 58 ++++++ .../Utilities/SQL/SQLLimitDetectorTests.swift | 99 ++++++++++ .../Utilities/SQL/SQLLimitInjectorTests.swift | 172 ------------------ docs/customization/editor-settings.mdx | 2 +- docs/features/sql-editor.mdx | 5 +- 16 files changed, 240 insertions(+), 352 deletions(-) delete mode 100644 TablePro/Core/Coordinators/QueryExecutionCoordinator+RowLimit.swift rename TablePro/Core/Utilities/SQL/{SQLLimitInjector.swift => SQLLimitDetector.swift} (57%) create mode 100644 TableProTests/Core/Utilities/SQL/SQLLimitDetectorTests.swift delete mode 100644 TableProTests/Core/Utilities/SQL/SQLLimitInjectorTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 486502426..1424ec1c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,9 +12,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - SQL Server connections can now use Windows Authentication (Kerberos) on macOS. Pick Windows Authentication in the connection form to sign in with the Kerberos ticket you already have from `kinit`, or enter a Kerberos principal and password to sign in with your domain credentials. Connect by hostname, not IP address. (#1879) - Teradata support through a downloadable driver written in native Swift. Connect over TD2 or TDNEGO logon, optionally with TLS, browse databases, tables, and columns, run SQL, edit rows, and create or alter tables. (#1867) +### Changed + +- The query result row cap no longer adds a LIMIT to the SQL it sends. Your query now runs exactly as you wrote it and TablePro stops reading once it reaches the cap, which is how SQL Server and Oracle already worked. Adding a LIMIT could change how the server planned a query with an ORDER BY, and return different rows than the query you typed. (#1884) + ### Fixed - Fixed MySQL and MariaDB queries ending in ORDER BY that could show an empty result with no error. A failure part way through reading rows was treated as a normal end of results, so a real server error looked like a table with no rows. TablePro now shows the error the server reported. (#1884) +- Fixed the query result row cap returning a single row when it was set to unlimited. (#1884) - Fixed SSH tunnels that could accept a database connection and then go quiet instead of forwarding it or failing, which showed up as MySQL and MariaDB connections timing out while reading the server greeting. A tunnel that cannot open its forwarding channel now gives up after 10 seconds, closes the connection, and logs the reason, instead of leaving the driver to wait out its own timeout with no explanation. (#1883) - Fixed connections being reset when a single database session opened several at once through one SSH tunnel, which could happen while browsing schemas. (#1883) diff --git a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift index 929faeafd..7ab9f248c 100644 --- a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift +++ b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift @@ -168,9 +168,6 @@ public protocol PluginDatabaseDriver: AnyObject, Sendable { // EXPLAIN query building (optional) func buildExplainQuery(_ sql: String) -> String? - // Row limit injection for executed queries (optional, return nil to use app-level fallback) - func injectRowLimit(_ sql: String, limit: Int) -> String? - func quoteIdentifier(_ name: String) -> String func escapeStringLiteral(_ value: String) -> String @@ -350,8 +347,6 @@ public extension PluginDatabaseDriver { func buildExplainQuery(_ sql: String) -> String? { nil } - func injectRowLimit(_ sql: String, limit: Int) -> String? { nil } - func createViewTemplate() -> String? { nil } func editViewFallbackTemplate(viewName: String) -> String? { nil } func castColumnToText(_ column: String) -> String { column } diff --git a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Helpers.swift b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Helpers.swift index 1b0a1b70c..be9c34b9e 100644 --- a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Helpers.swift +++ b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Helpers.swift @@ -11,8 +11,22 @@ import TableProPluginKit private let helpersLogger = Logger(subsystem: "com.TablePro", category: "QueryExecutionCoordinator") extension QueryExecutionCoordinator { - func resolveRowCap(sql: String, tabType: TabType) -> Int? { - QueryExecutor.resolveRowCap(sql: sql, tabType: tabType, databaseType: parent.connection.type) + func resolveRowCap(sql: String, tabType: TabType, bypassLimit: Bool = false) -> Int? { + guard !bypassLimit, + let cap = QueryExecutor.resolveRowCap( + sql: sql, tabType: tabType, databaseType: parent.connection.type + ) + else { + return nil + } + guard !SQLLimitDetector.hasExplicitRowLimit( + sql, + autoLimitStyle: PluginManager.shared.autoLimitStyle(for: parent.connection.type), + lexicalDialect: parent.sqlDialect + ) else { + return nil + } + return cap } func parseSchemaMetadata(_ schema: FetchedTableSchema) -> ParsedSchemaMetadata { diff --git a/TablePro/Core/Coordinators/QueryExecutionCoordinator+MultiStatement.swift b/TablePro/Core/Coordinators/QueryExecutionCoordinator+MultiStatement.swift index 3cce35618..741b736c0 100644 --- a/TablePro/Core/Coordinators/QueryExecutionCoordinator+MultiStatement.swift +++ b/TablePro/Core/Coordinators/QueryExecutionCoordinator+MultiStatement.swift @@ -12,13 +12,13 @@ extension QueryExecutionCoordinator { } func executeStatement( - plan: QueryLimitPlan, + rowCap: Int?, originalSQL: String, driver: DatabaseDriver, parameters: [Any?]? = nil ) async throws -> QueryResult { - if plan.rowCap != nil { - return try await driver.executeUserQuery(query: plan.executedSQL, rowCap: plan.rowCap, parameters: parameters) + if rowCap != nil { + return try await driver.executeUserQuery(query: originalSQL, rowCap: rowCap, parameters: parameters) } if let parameters { return try await driver.executeParameterized(query: originalSQL, parameters: parameters) diff --git a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift index 22bc9a44d..4c9a51b84 100644 --- a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift +++ b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift @@ -89,7 +89,7 @@ extension QueryExecutionCoordinator { let conn = parent.connection let tabId = parent.tabManager.tabs[index].id - let plan = resolveExecutionPlan(sql: sql, tabType: tab.tabType, bypassLimit: bypassRowLimit) + let rowCap = resolveRowCap(sql: sql, tabType: tab.tabType, bypassLimit: bypassRowLimit) let (tableName, isEditable) = parent.resolveTableEditability(tab: tab, sql: sql) let needsMetadataFetch: Bool @@ -112,9 +112,9 @@ extension QueryExecutionCoordinator { do { let fetchResult = try await parent.queryExecutor.executeQuery( - sql: plan.executedSQL, + sql: sql, parameters: parameters, - rowCap: plan.rowCap + rowCap: rowCap ) guard !Task.isCancelled else { @@ -178,7 +178,7 @@ extension QueryExecutionCoordinator { parent.toolbarState.setExecuting(false) if error is CancellationError || Task.isCancelled { return } guard capturedGeneration == parent.queryGeneration else { return } - handleQueryExecutionError(error, sql: plan.executedSQL, tabId: tabId, connection: conn) + handleQueryExecutionError(error, sql: sql, tabId: tabId, connection: conn) } } } @@ -278,10 +278,10 @@ extension QueryExecutionCoordinator { : SQLParameterExtractor.convertToNativeStyle(sql: stmtSQL, parameters: parameters, style: style) let statementSQL = conversion?.sql ?? stmtSQL - let plan = resolveExecutionPlan(sql: statementSQL, tabType: tabType, bypassLimit: bypassRowLimit) - failedSQL = plan.executedSQL + let rowCap = resolveRowCap(sql: statementSQL, tabType: tabType, bypassLimit: bypassRowLimit) + failedSQL = statementSQL let result = try await executeStatement( - plan: plan, + rowCap: rowCap, originalSQL: statementSQL, driver: driver, parameters: conversion?.values diff --git a/TablePro/Core/Coordinators/QueryExecutionCoordinator+RowLimit.swift b/TablePro/Core/Coordinators/QueryExecutionCoordinator+RowLimit.swift deleted file mode 100644 index 4163899b4..000000000 --- a/TablePro/Core/Coordinators/QueryExecutionCoordinator+RowLimit.swift +++ /dev/null @@ -1,39 +0,0 @@ -// -// QueryExecutionCoordinator+RowLimit.swift -// TablePro -// - -import Foundation -import TableProPluginKit - -struct QueryLimitPlan { - let rowCap: Int? - let executedSQL: String -} - -extension QueryExecutionCoordinator { - func resolveExecutionPlan(sql: String, tabType: TabType, bypassLimit: Bool = false) -> QueryLimitPlan { - guard !bypassLimit, let rowCap = resolveRowCap(sql: sql, tabType: tabType) else { - return QueryLimitPlan(rowCap: nil, executedSQL: sql) - } - let overFetchLimit = rowCap + 1 - if let adapter = DatabaseManager.shared.driver(for: parent.connectionId) as? PluginDriverAdapter, - let injected = adapter.injectRowLimit(sql, limit: overFetchLimit) { - return QueryLimitPlan(rowCap: rowCap, executedSQL: injected) - } - let injection = SQLLimitInjector.inject( - into: sql, - limit: overFetchLimit, - autoLimitStyle: PluginManager.shared.autoLimitStyle(for: parent.connection.type), - lexicalDialect: parent.sqlDialect - ) - switch injection { - case .injected(let injectedSQL): - return QueryLimitPlan(rowCap: rowCap, executedSQL: injectedSQL) - case .alreadyLimited: - return QueryLimitPlan(rowCap: nil, executedSQL: sql) - case .notInjectable: - return QueryLimitPlan(rowCap: rowCap, executedSQL: sql) - } - } -} diff --git a/TablePro/Core/Plugins/PluginDriverAdapter.swift b/TablePro/Core/Plugins/PluginDriverAdapter.swift index 67f745a2f..2dafa96a1 100644 --- a/TablePro/Core/Plugins/PluginDriverAdapter.swift +++ b/TablePro/Core/Plugins/PluginDriverAdapter.swift @@ -627,12 +627,6 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable { pluginDriver.buildExplainQuery(sql) } - // MARK: - Row Limit Injection - - func injectRowLimit(_ sql: String, limit: Int) -> String? { - pluginDriver.injectRowLimit(sql, limit: limit) - } - // MARK: - View Templates func createViewTemplate() -> String? { diff --git a/TablePro/Core/Services/Query/QueryExecutor.swift b/TablePro/Core/Services/Query/QueryExecutor.swift index 49efc0f2c..b739328e0 100644 --- a/TablePro/Core/Services/Query/QueryExecutor.swift +++ b/TablePro/Core/Services/Query/QueryExecutor.swift @@ -222,7 +222,8 @@ final class QueryExecutor { else { return nil } - return dataGridSettings.validatedQueryResultRowCap + let cap = dataGridSettings.validatedQueryResultRowCap + return cap > 0 ? cap : nil } static func qualifiesForRowCap(sql: String, tabType: TabType, databaseType: DatabaseType) -> Bool { diff --git a/TablePro/Core/Utilities/SQL/SQLLimitInjector.swift b/TablePro/Core/Utilities/SQL/SQLLimitDetector.swift similarity index 57% rename from TablePro/Core/Utilities/SQL/SQLLimitInjector.swift rename to TablePro/Core/Utilities/SQL/SQLLimitDetector.swift index 34061561d..951444fb4 100644 --- a/TablePro/Core/Utilities/SQL/SQLLimitInjector.swift +++ b/TablePro/Core/Utilities/SQL/SQLLimitDetector.swift @@ -1,74 +1,23 @@ // -// SQLLimitInjector.swift +// SQLLimitDetector.swift // TablePro // import Foundation import TableProPluginKit -enum SQLLimitInjectionResult: Equatable { - case injected(String) - case alreadyLimited - case notInjectable -} - -enum SQLLimitInjector { - static func inject( - into sql: String, - limit: Int, +enum SQLLimitDetector { + static func hasExplicitRowLimit( + _ sql: String, autoLimitStyle: AutoLimitStyle, lexicalDialect: SqlDialect - ) -> SQLLimitInjectionResult { - guard autoLimitStyle != .none, limit > 0 else { return .notInjectable } - switch scan(sql, dialect: lexicalDialect, autoLimitStyle: autoLimitStyle) { - case .insertAt(let index): - guard autoLimitStyle == .limit else { return .notInjectable } - var result = sql - result.insert(contentsOf: " LIMIT \(limit)", at: String.Index(utf16Offset: index, in: result)) - return .injected(result) - case .alreadyLimited: - return .alreadyLimited - case .notInjectable: - return .notInjectable - } - } - - private enum ScanResult { - case insertAt(Int) - case alreadyLimited - case notInjectable - } - - private static let limitingKeywords: [String] = ["LIMIT", "FETCH"] - private static let blockingKeywords: [String] = [ - "OFFSET", "FOR", "INTO", "LOCK", "FORMAT", "SETTINGS", "ALLOW", "FILTERING", - ] - - private static let singleQuote = UInt16(UnicodeScalar("'").value) - private static let doubleQuote = UInt16(UnicodeScalar("\"").value) - private static let backtick = UInt16(UnicodeScalar("`").value) - private static let semicolon = UInt16(UnicodeScalar(";").value) - private static let dash = UInt16(UnicodeScalar("-").value) - private static let slash = UInt16(UnicodeScalar("/").value) - private static let star = UInt16(UnicodeScalar("*").value) - private static let hash = UInt16(UnicodeScalar("#").value) - 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 openParen = UInt16(UnicodeScalar("(").value) - private static let closeParen = UInt16(UnicodeScalar(")").value) - - private static func scan( - _ sql: String, - dialect: SqlDialect, - autoLimitStyle: AutoLimitStyle - ) -> ScanResult { + ) -> Bool { let buffer = sql as NSString let length = buffer.length - guard length > 0 else { return .notInjectable } + guard length > 0 else { return false } - let dollarQuotesEnabled = dialect.supportsDollarQuotes - let hashCommentsEnabled = dialect.supportsHashLineComments + let dollarQuotesEnabled = lexicalDialect.supportsDollarQuotes + let hashCommentsEnabled = lexicalDialect.supportsHashLineComments var inString = false var stringChar: UInt16 = 0 var inLineComment = false @@ -76,8 +25,6 @@ enum SQLLimitInjector { var inDollarQuote = false var dollarTag = "" var parenDepth = 0 - var lastCodeEnd = 0 - var sawStatementEnd = false var i = 0 while i < length { @@ -103,12 +50,10 @@ enum SQLLimitInjector { if ch == dollar, SqlDollarQuote.matchesClose(at: i, tag: dollarTag, in: buffer, bufLen: length) { inDollarQuote = false i += (dollarTag as NSString).length + 2 - lastCodeEnd = i dollarTag = "" continue } i += 1 - lastCodeEnd = i continue } @@ -130,16 +75,8 @@ enum SQLLimitInjector { continue } - if isWhitespace(ch), !inString { - i += 1 - continue - } - - if sawStatementEnd { return .notInjectable } - if inString, ch == backslash, i + 1 < length { i += 2 - lastCodeEnd = i continue } @@ -150,19 +87,16 @@ enum SQLLimitInjector { } else if ch == stringChar { if i + 1 < length, buffer.character(at: i + 1) == stringChar { i += 2 - lastCodeEnd = i continue } inString = false } i += 1 - lastCodeEnd = i continue } if inString { i += 1 - lastCodeEnd = i continue } @@ -171,68 +105,59 @@ enum SQLLimitInjector { inDollarQuote = true dollarTag = tag i += openerLength - lastCodeEnd = i continue } if ch == openParen { parenDepth += 1 i += 1 - lastCodeEnd = i continue } if ch == closeParen { parenDepth -= 1 - guard parenDepth >= 0 else { return .notInjectable } i += 1 - lastCodeEnd = i continue } - if ch == semicolon, parenDepth == 0 { - sawStatementEnd = true - i += 1 - continue - } - - if SqlDollarQuote.isIdentifierStart(ch), i == 0 || !SqlDollarQuote.isIdentifierContinuation(buffer.character(at: i - 1)) { + if SqlDollarQuote.isIdentifierStart(ch), + i == 0 || !SqlDollarQuote.isIdentifierContinuation(buffer.character(at: i - 1)) { var end = i + 1 while end < length, SqlDollarQuote.isIdentifierPart(buffer.character(at: end)) { end += 1 } - if parenDepth == 0 { - if matchesAny(limitingKeywords, in: buffer, start: i, end: end) { - return .alreadyLimited - } - if autoLimitStyle == .top, matchesKeyword("TOP", in: buffer, start: i, end: end) { - return .alreadyLimited - } - if matchesAny(blockingKeywords, in: buffer, start: i, end: end) { - return .notInjectable - } + if parenDepth == 0, isLimitingKeyword( + in: buffer, start: i, end: end, autoLimitStyle: autoLimitStyle + ) { + return true } i = end - lastCodeEnd = end continue } i += 1 - lastCodeEnd = i } - guard !inString, !inBlockComment, !inDollarQuote, parenDepth == 0, lastCodeEnd > 0 else { - return .notInjectable - } - return .insertAt(lastCodeEnd) + return false } - private static func matchesAny(_ keywords: [String], in buffer: NSString, start: Int, end: Int) -> Bool { - keywords.contains { matchesKeyword($0, in: buffer, start: start, end: end) } + private static let limitingKeywords: [String] = ["LIMIT", "FETCH"] + + private static func isLimitingKeyword( + in buffer: NSString, + start: Int, + end: Int, + autoLimitStyle: AutoLimitStyle + ) -> Bool { + if limitingKeywords.contains(where: { matchesKeyword($0, in: buffer, start: start, end: end) }) { + return true + } + return autoLimitStyle == .top && matchesKeyword("TOP", in: buffer, start: start, end: end) } private static func matchesKeyword(_ keyword: String, in buffer: NSString, start: Int, end: Int) -> Bool { let keywordBuffer = keyword as NSString guard end - start == keywordBuffer.length else { return false } - for offset in 0..= 0x61 && ch <= 0x7A) ? ch - 0x20 : ch } - private static func isWhitespace(_ ch: UInt16) -> Bool { - ch == 0x20 || ch == 0x09 || ch == 0x0A || ch == 0x0D - } + private static let singleQuote = UInt16(UnicodeScalar("'").value) + private static let doubleQuote = UInt16(UnicodeScalar("\"").value) + private static let backtick = UInt16(UnicodeScalar("`").value) + private static let dash = UInt16(UnicodeScalar("-").value) + private static let slash = UInt16(UnicodeScalar("/").value) + private static let star = UInt16(UnicodeScalar("*").value) + private static let hash = UInt16(UnicodeScalar("#").value) + 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 openParen = UInt16(UnicodeScalar("(").value) + private static let closeParen = UInt16(UnicodeScalar(")").value) } diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift index ef9f9d86b..c1943f40e 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+QueryHelpers.swift @@ -28,8 +28,8 @@ extension MainContentCoordinator { } } - func resolveExecutionPlan(sql: String, tabType: TabType, bypassLimit: Bool = false) -> QueryLimitPlan { - queryExecutionCoordinator.resolveExecutionPlan(sql: sql, tabType: tabType, bypassLimit: bypassLimit) + func resolveRowCap(sql: String, tabType: TabType, bypassLimit: Bool = false) -> Int? { + queryExecutionCoordinator.resolveRowCap(sql: sql, tabType: tabType, bypassLimit: bypassLimit) } func parseSchemaMetadata(_ schema: FetchedTableSchema) -> ParsedSchemaMetadata { diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 78b546332..0722b3e31 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -1164,7 +1164,7 @@ final class MainContentCoordinator { let conn = connection let tabId = tabManager.tabs[index].id - let plan = resolveExecutionPlan(sql: sql, tabType: tab.tabType, bypassLimit: bypassRowLimit) + let rowCap = resolveRowCap(sql: sql, tabType: tab.tabType, bypassLimit: bypassRowLimit) let (tableName, isEditable) = resolveTableEditability(tab: tab, sql: sql) let needsMetadataFetch: Bool @@ -1217,9 +1217,9 @@ final class MainContentCoordinator { do { let fetchResult = try await queryExecutor.executeQuery( - sql: plan.executedSQL, + sql: sql, parameters: nil, - rowCap: plan.rowCap + rowCap: rowCap ) guard !Task.isCancelled else { @@ -1305,7 +1305,7 @@ final class MainContentCoordinator { pendingLoadTrigger = trigger return } - handleQueryExecutionError(error, sql: plan.executedSQL, tabId: tabId, connection: conn) + handleQueryExecutionError(error, sql: sql, tabId: tabId, connection: conn) } } } diff --git a/TableProTests/Core/Services/Query/QueryExecutorTests.swift b/TableProTests/Core/Services/Query/QueryExecutorTests.swift index db0c477b5..23dce84c6 100644 --- a/TableProTests/Core/Services/Query/QueryExecutorTests.swift +++ b/TableProTests/Core/Services/Query/QueryExecutorTests.swift @@ -165,6 +165,64 @@ struct QueryExecutorTests { )) } + @Test("qualifiesForRowCap accepts a SELECT ending in ORDER BY") + func qualifiesForRowCapOrderBy() { + #expect(QueryExecutor.qualifiesForRowCap( + sql: "SELECT * FROM alert_events ORDER BY alert_time", tabType: .query, databaseType: .mysql + )) + #expect(QueryExecutor.qualifiesForRowCap( + sql: "SELECT * FROM t WHERE created_at >= '2026-01-01' ORDER BY id DESC", + tabType: .query, databaseType: .mysql + )) + } + + // MARK: - Row cap resolution + + private func withRowCapSettings(truncate: Bool, cap: Int, _ body: () -> Void) { + let previousTruncate = AppSettingsManager.shared.dataGrid.truncateQueryResults + let previousCap = AppSettingsManager.shared.dataGrid.queryResultRowCap + AppSettingsManager.shared.dataGrid.truncateQueryResults = truncate + AppSettingsManager.shared.dataGrid.queryResultRowCap = cap + defer { + AppSettingsManager.shared.dataGrid.truncateQueryResults = previousTruncate + AppSettingsManager.shared.dataGrid.queryResultRowCap = previousCap + } + body() + } + + @Test("resolveRowCap returns nil when the row cap is unlimited") + func resolveRowCapUnlimitedReturnsNil() { + withRowCapSettings(truncate: true, cap: 0) { + #expect(QueryExecutor.resolveRowCap( + sql: "SELECT * FROM alert_events ORDER BY alert_time", + tabType: .query, + databaseType: .mysql + ) == nil) + } + } + + @Test("resolveRowCap returns the configured cap for a capped SELECT") + func resolveRowCapReturnsConfiguredCap() { + withRowCapSettings(truncate: true, cap: 10_000) { + #expect(QueryExecutor.resolveRowCap( + sql: "SELECT * FROM alert_events ORDER BY alert_time", + tabType: .query, + databaseType: .mysql + ) == 10_000) + } + } + + @Test("resolveRowCap returns nil when truncation is disabled") + func resolveRowCapTruncationDisabled() { + withRowCapSettings(truncate: false, cap: 10_000) { + #expect(QueryExecutor.resolveRowCap( + sql: "SELECT * FROM users", + tabType: .query, + databaseType: .mysql + ) == nil) + } + } + // MARK: - Parameter detection @Test("detectAndReconcileParameters returns empty when SQL has no placeholders") diff --git a/TableProTests/Core/Utilities/SQL/SQLLimitDetectorTests.swift b/TableProTests/Core/Utilities/SQL/SQLLimitDetectorTests.swift new file mode 100644 index 000000000..50a13fcb3 --- /dev/null +++ b/TableProTests/Core/Utilities/SQL/SQLLimitDetectorTests.swift @@ -0,0 +1,99 @@ +// +// SQLLimitDetectorTests.swift +// TableProTests +// + +import Foundation +import TableProPluginKit +import Testing + +@testable import TablePro + +@Suite("SQLLimitDetector") +struct SQLLimitDetectorTests { + private func hasLimit( + _ sql: String, + style: AutoLimitStyle = .limit, + dialect: SqlDialect = .generic + ) -> Bool { + SQLLimitDetector.hasExplicitRowLimit(sql, autoLimitStyle: style, lexicalDialect: dialect) + } + + @Test("A bare SELECT has no explicit row limit") + func bareSelect() { + #expect(!hasLimit("SELECT * FROM users")) + } + + @Test("A SELECT ending in ORDER BY has no explicit row limit") + func orderByIsNotALimit() { + #expect(!hasLimit("SELECT * FROM alert_events ORDER BY alert_time")) + #expect(!hasLimit("SELECT *\nFROM alert_events ORDER BY alert_time")) + #expect(!hasLimit("SELECT * FROM t WHERE created_at >= '2026-01-01' ORDER BY id DESC")) + #expect(!hasLimit("SELECT * FROM t ORDER BY a, b DESC")) + } + + @Test("Detects a top-level LIMIT") + func topLevelLimit() { + #expect(hasLimit("SELECT * FROM users LIMIT 10")) + #expect(hasLimit("select * from users limit 10 offset 5")) + #expect(hasLimit("SELECT * FROM t ORDER BY id DESC LIMIT 10")) + } + + @Test("Detects FETCH FIRST") + func fetchFirst() { + #expect(hasLimit("SELECT * FROM t FETCH FIRST 5 ROWS ONLY")) + } + + @Test("Detects TOP only under the top style") + func topOnlyForTopStyle() { + #expect(hasLimit("SELECT TOP 5 * FROM t", style: .top)) + #expect(!hasLimit("SELECT top FROM quotas")) + } + + @Test("A top-level OFFSET without LIMIT is not an explicit row limit") + func offsetWithoutLimit() { + #expect(!hasLimit("SELECT * FROM users OFFSET 5")) + } + + @Test("An inner LIMIT in a CTE or subquery does not bound the outer statement") + func innerLimitDoesNotCount() { + #expect(!hasLimit("WITH cte AS (SELECT * FROM t LIMIT 5) SELECT * FROM cte")) + #expect(!hasLimit("SELECT * FROM (SELECT * FROM t LIMIT 5) sub")) + #expect(!hasLimit("(SELECT a FROM t1 LIMIT 5) UNION (SELECT b FROM t2 LIMIT 5)")) + } + + @Test("Detects a LIMIT that applies to a whole UNION") + func unionLimit() { + #expect(hasLimit("SELECT a FROM t1 UNION SELECT b FROM t2 LIMIT 5")) + #expect(!hasLimit("SELECT a FROM t1 UNION ALL SELECT b FROM t2")) + } + + @Test("Ignores LIMIT-like text inside comments") + func ignoresComments() { + #expect(!hasLimit("SELECT * FROM t -- fetch it all")) + #expect(!hasLimit("SELECT * FROM t /* LIMIT 9 */")) + #expect(!hasLimit("SELECT * FROM t # see LIMIT docs", dialect: .mysql)) + } + + @Test("Ignores LIMIT-like text inside string literals") + func ignoresStringLiterals() { + #expect(!hasLimit("SELECT * FROM t WHERE note = 'no LIMIT here'")) + } + + @Test("Ignores LIMIT inside dollar-quoted bodies for PostgreSQL") + func ignoresDollarQuotedBodies() { + #expect(!hasLimit("SELECT $tag$LIMIT 5$tag$ FROM t", dialect: .postgres)) + } + + @Test("Does not mistake identifiers containing limit for a LIMIT clause") + func identifiersContainingLimit() { + #expect(!hasLimit("SELECT limit_used FROM quotas")) + #expect(!hasLimit("SELECT `limit` FROM quotas")) + } + + @Test("Detects a LIMIT after a trailing semicolon is stripped") + func trailingSemicolon() { + #expect(hasLimit("SELECT * FROM t LIMIT 5;")) + #expect(!hasLimit("SELECT * FROM t;")) + } +} diff --git a/TableProTests/Core/Utilities/SQL/SQLLimitInjectorTests.swift b/TableProTests/Core/Utilities/SQL/SQLLimitInjectorTests.swift deleted file mode 100644 index 808be598f..000000000 --- a/TableProTests/Core/Utilities/SQL/SQLLimitInjectorTests.swift +++ /dev/null @@ -1,172 +0,0 @@ -// -// SQLLimitInjectorTests.swift -// TableProTests -// - -import Foundation -import TableProPluginKit -import Testing -@testable import TablePro - -@Suite("SQLLimitInjector appends a LIMIT only when the statement has none") -struct SQLLimitInjectorTests { - private func inject( - _ sql: String, - limit: Int = 501, - style: AutoLimitStyle = .limit, - dialect: SqlDialect = .generic - ) -> SQLLimitInjectionResult { - SQLLimitInjector.inject(into: sql, limit: limit, autoLimitStyle: style, lexicalDialect: dialect) - } - - @Test("Appends LIMIT to a bare SELECT") - func appendsToBareSelect() { - #expect(inject("SELECT * FROM users") == .injected("SELECT * FROM users LIMIT 501")) - } - - @Test("Reports alreadyLimited for a top-level LIMIT") - func detectsExistingLimit() { - #expect(inject("SELECT * FROM users LIMIT 10") == .alreadyLimited) - #expect(inject("select * from users limit 10 offset 5") == .alreadyLimited) - } - - @Test("Reports alreadyLimited for FETCH FIRST") - func detectsFetchFirst() { - #expect(inject("SELECT * FROM t FETCH FIRST 5 ROWS ONLY") == .alreadyLimited) - } - - @Test("Reports alreadyLimited for TOP only under the top style") - func detectsTopOnlyForTopStyle() { - #expect(inject("SELECT TOP 5 * FROM t", style: .top) == .alreadyLimited) - #expect(inject("SELECT top FROM quotas") == .injected("SELECT top FROM quotas LIMIT 501")) - } - - @Test("Reports notInjectable for a top-level OFFSET without LIMIT") - func offsetAloneIsNotInjectable() { - #expect(inject("SELECT * FROM users OFFSET 5") == .notInjectable) - } - - @Test("Injects on the outer statement when a CTE has an inner LIMIT") - func injectsOuterStatementForCte() { - let sql = "WITH cte AS (SELECT * FROM t LIMIT 5) SELECT * FROM cte" - #expect(inject(sql) == .injected("WITH cte AS (SELECT * FROM t LIMIT 5) SELECT * FROM cte LIMIT 501")) - } - - @Test("Injects when only a subquery has a LIMIT") - func injectsWhenSubqueryLimited() { - let sql = "SELECT * FROM (SELECT * FROM t LIMIT 5) sub" - #expect(inject(sql) == .injected("SELECT * FROM (SELECT * FROM t LIMIT 5) sub LIMIT 501")) - } - - @Test("Inserts before a trailing line comment") - func insertsBeforeTrailingLineComment() { - #expect(inject("SELECT * FROM t -- fetch it all") == .injected("SELECT * FROM t LIMIT 501 -- fetch it all")) - #expect(inject("SELECT * FROM t\n-- done") == .injected("SELECT * FROM t LIMIT 501\n-- done")) - } - - @Test("Inserts before a trailing block comment") - func insertsBeforeTrailingBlockComment() { - #expect(inject("SELECT * FROM t /* LIMIT 9 */") == .injected("SELECT * FROM t LIMIT 501 /* LIMIT 9 */")) - } - - @Test("Keeps a leading comment and injects at the end") - func keepsLeadingComment() { - #expect(inject("-- top users\nSELECT * FROM users") == .injected("-- top users\nSELECT * FROM users LIMIT 501")) - } - - @Test("Treats hash comments as comments only for MySQL") - func hashCommentsAreDialectGated() { - #expect(inject("SELECT * FROM t # note", dialect: .mysql) == .injected("SELECT * FROM t LIMIT 501 # note")) - #expect(inject("SELECT * FROM t # see LIMIT docs", dialect: .mysql) - == .injected("SELECT * FROM t LIMIT 501 # see LIMIT docs")) - #expect(inject("SELECT 1 # 2", dialect: .postgres) == .injected("SELECT 1 # 2 LIMIT 501")) - } - - @Test("Reports notInjectable for Cassandra ALLOW FILTERING") - func allowFilteringIsNotInjectable() { - #expect(inject("SELECT * FROM t WHERE x = 1 ALLOW FILTERING") == .notInjectable) - } - - @Test("Appends once after a UNION without a top-level LIMIT") - func appendsAfterUnion() { - let sql = "SELECT a FROM t1 UNION ALL SELECT b FROM t2" - #expect(inject(sql) == .injected("SELECT a FROM t1 UNION ALL SELECT b FROM t2 LIMIT 501")) - } - - @Test("Reports alreadyLimited when a LIMIT applies to the whole UNION") - func detectsUnionTopLevelLimit() { - #expect(inject("SELECT a FROM t1 UNION SELECT b FROM t2 LIMIT 5") == .alreadyLimited) - } - - @Test("Appends after a parenthesized UNION whose branches have inner LIMITs") - func appendsAfterParenthesizedUnion() { - let sql = "(SELECT a FROM t1 LIMIT 5) UNION (SELECT b FROM t2 LIMIT 5)" - #expect(inject(sql) == .injected("(SELECT a FROM t1 LIMIT 5) UNION (SELECT b FROM t2 LIMIT 5) LIMIT 501")) - } - - @Test("Preserves a trailing semicolon after the injected clause") - func preservesTrailingSemicolon() { - #expect(inject("SELECT * FROM t;") == .injected("SELECT * FROM t LIMIT 501;")) - #expect(inject("SELECT * FROM t; -- done") == .injected("SELECT * FROM t LIMIT 501; -- done")) - } - - @Test("Ignores LIMIT-like text inside string literals") - func ignoresLimitInsideStrings() { - #expect(inject("SELECT * FROM t WHERE note = 'no LIMIT here'") - == .injected("SELECT * FROM t WHERE note = 'no LIMIT here' LIMIT 501")) - } - - @Test("Ignores LIMIT inside dollar-quoted bodies for PostgreSQL") - func ignoresLimitInsideDollarQuotes() { - let sql = "SELECT $tag$LIMIT 5$tag$ FROM t" - #expect(inject(sql, dialect: .postgres) == .injected("SELECT $tag$LIMIT 5$tag$ FROM t LIMIT 501")) - } - - @Test("Does not mistake identifiers containing limit for a LIMIT clause") - func ignoresLimitLikeIdentifiers() { - #expect(inject("SELECT limit_used FROM quotas") == .injected("SELECT limit_used FROM quotas LIMIT 501")) - #expect(inject("SELECT `limit` FROM quotas") == .injected("SELECT `limit` FROM quotas LIMIT 501")) - } - - @Test("Reports notInjectable for trailing clauses that must not precede LIMIT") - func trailingClausesAreNotInjectable() { - #expect(inject("SELECT * FROM t FOR UPDATE") == .notInjectable) - #expect(inject("SELECT * FROM t LOCK IN SHARE MODE") == .notInjectable) - #expect(inject("SELECT * INTO backup FROM t") == .notInjectable) - #expect(inject("SELECT * FROM t FORMAT JSON") == .notInjectable) - #expect(inject("SELECT * FROM t SETTINGS max_threads = 1") == .notInjectable) - } - - @Test("Reports notInjectable for non-limit dialect styles") - func nonLimitStylesAreNotInjectable() { - #expect(inject("SELECT * FROM t", style: .top) == .notInjectable) - #expect(inject("SELECT * FROM t", style: .fetchFirst) == .notInjectable) - #expect(inject("SELECT * FROM t", style: AutoLimitStyle.none) == .notInjectable) - } - - @Test("Detects an existing constraint even for non-limit dialect styles") - func detectsConstraintForNonLimitStyles() { - #expect(inject("SELECT * FROM t FETCH FIRST 5 ROWS ONLY", style: .fetchFirst) == .alreadyLimited) - } - - @Test("Reports notInjectable for non-positive limits, empty input, and unbalanced statements") - func invalidInputIsNotInjectable() { - #expect(inject("SELECT * FROM t", limit: 0) == .notInjectable) - #expect(inject("") == .notInjectable) - #expect(inject(" ") == .notInjectable) - #expect(inject("-- only a comment") == .notInjectable) - #expect(inject("SELECT * FROM (t") == .notInjectable) - #expect(inject("SELECT 'unterminated FROM t") == .notInjectable) - } - - @Test("Reports notInjectable when code follows a top-level semicolon") - func multiStatementInputIsNotInjectable() { - #expect(inject("SELECT 1; SELECT 2") == .notInjectable) - } - - @Test("Handles escaped quotes inside string literals") - func handlesEscapedQuotes() { - #expect(inject("SELECT * FROM t WHERE name = 'it''s'") - == .injected("SELECT * FROM t WHERE name = 'it''s' LIMIT 501")) - } -} diff --git a/docs/customization/editor-settings.mdx b/docs/customization/editor-settings.mdx index 86176a3d7..b5bd643bd 100644 --- a/docs/customization/editor-settings.mdx +++ b/docs/customization/editor-settings.mdx @@ -65,4 +65,4 @@ Tables with more estimated rows than the threshold show an approximate count ins | Truncate query results | On | | | Row cap | 10,000 | 100; 1,000; 5,000; 10,000; 50,000; 100,000; 500,000 | -When on, `SELECT` queries without their own `LIMIT` run with the cap applied. The query text in the editor never changes. Capped results show a **Fetch All** button, and **Execute Without Limit** skips the cap for one run. +When on, `SELECT` queries without their own `LIMIT` run with the cap applied to the rows TablePro reads back. The SQL sent to the server is always the query you wrote. Capped results show a **Fetch All** button, and **Execute Without Limit** skips the cap for one run. diff --git a/docs/features/sql-editor.mdx b/docs/features/sql-editor.mdx index 74b3abe1b..8f06ec20e 100644 --- a/docs/features/sql-editor.mdx +++ b/docs/features/sql-editor.mdx @@ -69,10 +69,9 @@ Match modes: contains, matches word, starts with, ends with, or regular expressi SELECT and WITH queries without their own `LIMIT` or `FETCH FIRST` run with the configured row cap: - The default cap is 10,000 rows. Pick 100 to 500,000 in **Settings > Editor** (**Row cap**), or turn off **Truncate query results** to disable it. -- On databases whose SQL uses `LIMIT`, TablePro appends a `LIMIT` to the statement it sends, so the server stops early instead of returning millions of rows. The editor text and query history keep the query as you wrote it. -- Your own `LIMIT` or `FETCH FIRST` always wins: the query is sent as written and not capped for that run. +- Your query is sent exactly as you wrote it. TablePro never changes the SQL, it stops reading once it reaches the cap. +- Your own `LIMIT`, `FETCH FIRST`, or `TOP` always wins: the query is not capped for that run. - `EXPLAIN`, `SHOW`, writes, and DDL are never limited. -- Statements ending in clauses that must follow `LIMIT` (such as `FOR UPDATE` or CQL's `ALLOW FILTERING`) are sent as written, and SQL Server and Oracle queries run unmodified; in these cases the cap is enforced on the result after the query runs. When the cap trims a result, the status bar shows **truncated** with a **Fetch All** link. To skip the cap for one run, press `Cmd+Option+Enter` or use the Execute button's menu or the Query menu.