fix(plugin-mysql): report the real error when a query fails while reading rows (#1884)

CHANGELOG.md

@@ -12,8 +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)
 

Plugins/MySQLDriverPlugin/CMariaDB/CMariaDB.h

@@ -10,5 +10,6 @@
 #define CMariaDB_h
 
 #include "include/mysql.h"
+#include "include/mysqld_error.h"
 
 #endif /* CMariaDB_h */

Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift

@@ -67,7 +67,7 @@
 func mysqlTypeToString(_ fieldPtr: UnsafePointer<MYSQL_FIELD>) -> String {
     let field = fieldPtr.pointee
     let flags = UInt(field.flags)
     let length = field.length
 
     // MariaDB extended metadata: detect JSON stored as LONGTEXT.
     // `MARIADB_CONST_STRING` is length-prefixed (not null-terminated), so we must read
@@ -381,14 +381,19 @@
         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)
         guard let killConn = killConn else { return }
 
         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 @@
         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 @@
         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 @@
 
         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 @@
 
         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()
             }
 

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 }

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 {

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)

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

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? {

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 {