fix(plugin-clickhouse): show the result grid whenever the server returns rows (#1896)

Author: datlechin

CHANGELOG.md

@@ -18,6 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Fixed ClickHouse queries showing only a success message with no result table. TablePro guessed whether a query returns data from its first word, which failed for queries starting with a comment or with SELECT on its own line. It now reads what the server actually returned, so any query that produces a result set shows the grid, including empty results and queries with their own FORMAT clause. (#1886)
+- Fixed ClickHouse INSERT statements always reporting 0 rows affected. (#1886)
+- Fixed ClickHouse query exports writing an empty file when the query started with a comment. (#1886)
 - Fixed a restored window tab sometimes showing a blank name in the tab bar until you switched to it. (#1894)
 - Fixed the window title not updating right away after saving a query to a file or opening a favorite into the current tab. (#1894)
 - Fixed the SQL editor in a new tab losing keyboard focus after the first couple of characters, which stopped further typing until you clicked back into the editor. (#1885)

Plugins/ClickHouseDriverPlugin/ClickHouseCapabilities.swift

@@ -15,6 +15,10 @@ struct ClickHouseCapabilities: Sendable, Equatable {
         major > 19 || (major == 19 && minor >= 17)
     }
 
+    var hasWriteExceptionInOutputFormatSetting: Bool {
+        major > 23 || (major == 23 && minor >= 8)
+    }
+
     static func parse(_ version: String?) -> ClickHouseCapabilities {
         guard let version else { return .unknown }
         let parts = version.split(separator: ".")

Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift

@@ -149,10 +149,6 @@ final class ClickHousePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
 
     static let logger = Logger(subsystem: "com.TablePro", category: "ClickHousePluginDriver")
 
-    static let selectPrefixes: Set<String> = [
-        "SELECT", "SHOW", "DESCRIBE", "DESC", "EXISTS", "EXPLAIN", "WITH"
-    ]
-
     var serverVersion: String? { _serverVersion }
     var supportsSchemas: Bool { false }
     var supportsTransactions: Bool { false }
@@ -604,9 +600,8 @@ final class ClickHousePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
         if !database.isEmpty {
             queryItems.append(URLQueryItem(name: "database", value: database))
         }
-        if !queryItems.isEmpty {
-            components.queryItems = queryItems
-        }
+        queryItems.append(URLQueryItem(name: "default_format", value: "JSONEachRow"))
+        components.queryItems = queryItems
 
         guard let url = components.url else {
             throw ClickHouseError(message: "Failed to construct request URL")
@@ -622,7 +617,7 @@ final class ClickHousePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
             request.setValue(authorization, forHTTPHeaderField: "Authorization")
         }
 
-        request.httpBody = (query + " FORMAT JSONEachRow").data(using: .utf8)
+        request.httpBody = query.data(using: .utf8)
         return request
     }
 

Plugins/ClickHouseDriverPlugin/ClickHousePluginDriver+Http.swift

@@ -24,50 +24,7 @@ extension ClickHousePluginDriver {
 
         var request = try buildRequest(query: query, database: database, queryId: queryId)
         request.timeoutInterval = _queryTimeout.requestTimeoutInterval
-        let isSelect = Self.isSelectLikeQuery(query)
-
-        let (data, response) = try await withTaskCancellationHandler {
-            try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<(Data, URLResponse), Error>) in
-                let task = session.dataTask(with: request) { data, response, error in
-                    if let error {
-                        continuation.resume(throwing: error)
-                        return
-                    }
-                    guard let data, let response else {
-                        continuation.resume(throwing: ClickHouseError(message: "Empty response from server"))
-                        return
-                    }
-                    continuation.resume(returning: (data, response))
-                }
-
-                self.lock.lock()
-                self.currentTask = task
-                self.lock.unlock()
-
-                task.resume()
-            }
-        } onCancel: {
-            self.lock.lock()
-            self.currentTask?.cancel()
-            self.currentTask = nil
-            self.lock.unlock()
-        }
-
-        lock.lock()
-        currentTask = nil
-        lock.unlock()
-
-        if let httpResponse = response as? HTTPURLResponse, httpResponse.statusCode >= 400 {
-            let body = String(data: data, encoding: .utf8) ?? "Unknown error"
-            Self.logger.error("ClickHouse HTTP \(httpResponse.statusCode): \(body)")
-            throw ClickHouseError(message: body.trimmingCharacters(in: .whitespacesAndNewlines))
-        }
-
-        if isSelect {
-            return parseTabSeparatedResponse(data)
-        }
-
-        return CHQueryResult(columns: [], columnTypeNames: [], rows: [], affectedRows: 0, isTruncated: false)
+        return try await perform(request: request, session: session)
     }
 
     func executeRawWithParams(_ query: String, params: [String: String?], queryId: String? = nil) async throws -> CHQueryResult {
@@ -84,9 +41,39 @@ extension ClickHousePluginDriver {
 
         var request = try buildRequest(query: query, database: database, queryId: queryId, params: params)
         request.timeoutInterval = _queryTimeout.requestTimeoutInterval
-        let isSelect = Self.isSelectLikeQuery(query)
+        return try await perform(request: request, session: session)
+    }
+
+    private func perform(request: URLRequest, session: URLSession) async throws -> CHQueryResult {
+        let (data, response) = try await send(request: request, session: session)
+
+        lock.lock()
+        currentTask = nil
+        lock.unlock()
+
+        let httpResponse = response as? HTTPURLResponse
+        if let httpResponse, httpResponse.statusCode >= 400 {
+            let body = String(data: data, encoding: .utf8) ?? "Unknown error"
+            let exceptionCode = httpResponse.value(forHTTPHeaderField: "X-ClickHouse-Exception-Code") ?? "none"
+            Self.logger.error("ClickHouse HTTP \(httpResponse.statusCode) exception \(exceptionCode): \(body)")
+            throw ClickHouseError(message: body.trimmingCharacters(in: .whitespacesAndNewlines))
+        }
+
+        let outcome = ClickHouseResponseClassifier.classify(
+            headers: Self.headerFields(httpResponse),
+            body: data
+        )
+        return CHQueryResult(
+            columns: outcome.columns,
+            columnTypeNames: outcome.columnTypeNames,
+            rows: outcome.rows,
+            affectedRows: outcome.affectedRows,
+            isTruncated: outcome.isTruncated
+        )
+    }
 
-        let (data, response) = try await withTaskCancellationHandler {
+    private func send(request: URLRequest, session: URLSession) async throws -> (Data, URLResponse) {
+        try await withTaskCancellationHandler {
             try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<(Data, URLResponse), Error>) in
                 let task = session.dataTask(with: request) { data, response, error in
                     if let error {
@@ -99,9 +86,11 @@ extension ClickHousePluginDriver {
                     }
                     continuation.resume(returning: (data, response))
                 }
+
                 self.lock.lock()
                 self.currentTask = task
                 self.lock.unlock()
+
                 task.resume()
             }
         } onCancel: {
@@ -110,22 +99,16 @@ extension ClickHousePluginDriver {
             self.currentTask = nil
             self.lock.unlock()
         }
+    }
 
-        lock.lock()
-        currentTask = nil
-        lock.unlock()
-
-        if let httpResponse = response as? HTTPURLResponse, httpResponse.statusCode >= 400 {
-            let body = String(data: data, encoding: .utf8) ?? "Unknown error"
-            Self.logger.error("ClickHouse HTTP \(httpResponse.statusCode): \(body)")
-            throw ClickHouseError(message: body.trimmingCharacters(in: .whitespacesAndNewlines))
-        }
-
-        if isSelect {
-            return parseTabSeparatedResponse(data)
+    private static func headerFields(_ response: HTTPURLResponse?) -> [String: String] {
+        guard let response else { return [:] }
+        var fields: [String: String] = [:]
+        for (key, value) in response.allHeaderFields {
+            guard let name = key as? String, let text = value as? String else { continue }
+            fields[name] = text
         }
-
-        return CHQueryResult(columns: [], columnTypeNames: [], rows: [], affectedRows: 0, isTruncated: false)
+        return fields
     }
 
     func buildRequest(query: String, database: String, queryId: String? = nil, params: [String: String?]? = nil) throws -> URLRequest {
@@ -145,14 +128,15 @@ extension ClickHousePluginDriver {
             queryItems.append(URLQueryItem(name: "query_id", value: queryId))
         }
         queryItems.append(URLQueryItem(name: "send_progress_in_http_headers", value: "1"))
+        queryItems.append(contentsOf: ClickHouseResponseClassifier.transportQueryItems(
+            supportsWriteExceptionSetting: ClickHouseCapabilities.parse(serverVersion).hasWriteExceptionInOutputFormatSetting
+        ))
         if let params {
             for (key, value) in params.sorted(by: { $0.key < $1.key }) {
                 queryItems.append(URLQueryItem(name: "param_\(key)", value: value))
             }
         }
-        if !queryItems.isEmpty {
-            components.queryItems = queryItems
-        }
+        components.queryItems = queryItems
 
         guard let url = components.url else {
             throw ClickHouseError(message: "Failed to construct request URL")
@@ -170,94 +154,11 @@ extension ClickHousePluginDriver {
 
         let trimmedQuery = query.trimmingCharacters(in: .whitespacesAndNewlines)
             .replacingOccurrences(of: ";+$", with: "", options: .regularExpression)
-
-        if Self.isSelectLikeQuery(trimmedQuery) {
-            request.httpBody = (trimmedQuery + " FORMAT TabSeparatedWithNamesAndTypes").data(using: .utf8)
-        } else {
-            request.httpBody = trimmedQuery.data(using: .utf8)
-        }
+        request.httpBody = trimmedQuery.data(using: .utf8)
 
         return request
     }
 
-    static func isSelectLikeQuery(_ query: String) -> Bool {
-        let trimmed = query.trimmingCharacters(in: .whitespacesAndNewlines)
-        guard let firstWord = trimmed.split(separator: " ", maxSplits: 1).first else {
-            return false
-        }
-        return selectPrefixes.contains(firstWord.uppercased())
-    }
-
-    func parseTabSeparatedResponse(_ data: Data) -> CHQueryResult {
-        guard let text = String(data: data, encoding: .utf8), !text.isEmpty else {
-            return CHQueryResult(columns: [], columnTypeNames: [], rows: [], affectedRows: 0, isTruncated: false)
-        }
-
-        let lines = text.components(separatedBy: "\n")
-
-        guard lines.count >= 2 else {
-            return CHQueryResult(columns: [], columnTypeNames: [], rows: [], affectedRows: 0, isTruncated: false)
-        }
-
-        let columns = lines[0].components(separatedBy: "\t")
-        let columnTypes = lines[1].components(separatedBy: "\t")
-
-        var rows: [[PluginCellValue]] = []
-        var truncated = false
-        for i in 2..<lines.count {
-            let line = lines[i]
-            if line.isEmpty { continue }
-
-            let fields = line.components(separatedBy: "\t")
-            let row: [PluginCellValue] = fields.map { field in
-                if field == "\\N" {
-                    return .null
-                }
-                return .text(Self.unescapeTsvField(field))
-            }
-            rows.append(row)
-            if rows.count >= PluginRowLimits.emergencyMax {
-                truncated = true
-                break
-            }
-        }
-
-        return CHQueryResult(
-            columns: columns,
-            columnTypeNames: columnTypes,
-            rows: rows,
-            affectedRows: rows.count,
-            isTruncated: truncated
-        )
-    }
-
-    static func unescapeTsvField(_ field: String) -> String {
-        var result = ""
-        result.reserveCapacity((field as NSString).length)
-        var iterator = field.makeIterator()
-
-        while let char = iterator.next() {
-            if char == "\\" {
-                if let next = iterator.next() {
-                    switch next {
-                    case "\\": result.append("\\")
-                    case "t": result.append("\t")
-                    case "n": result.append("\n")
-                    default:
-                        result.append("\\")
-                        result.append(next)
-                    }
-                } else {
-                    result.append("\\")
-                }
-            } else {
-                result.append(char)
-            }
-        }
-
-        return result
-    }
-
     /// Convert `?` placeholders to `{p1:String}` and build parameter map for ClickHouse HTTP params.
     static func buildClickHouseParams(
         query: String,
@@ -307,5 +208,4 @@ extension ClickHousePluginDriver {
 
         return (converted, paramMap)
     }
-
 }