Skip to content

Commit 2496e4e

Browse files
authored
fix(plugin-snowflake): key the shared session on the saved connection and scope Stop to its own driver (#2470)
Claude-Session: https://claude.ai/code/session_01Qk1xfY3vnneRifC22eV2r7
1 parent e2eb1a4 commit 2496e4e

7 files changed

Lines changed: 150 additions & 21 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
6161
- Snowflake `BINARY` cells showing the hex of their hex, which the hex editor then wrote back.
6262
- A date cell the app cannot read opening a picker set to today, which overwrote the value on OK. (#2454)
6363
- Snowflake reporting no rows changed for an `UPDATE` or `MERGE`, and a `SELECT`'s own result for a column named `number of rows`.
64+
- Two saved Snowflake connections to one account sharing a session, so switching database in one window moved the other.
65+
- Stop on a Snowflake query cancelling every other query on the same connection, including a sidebar refresh or a save.
6466

6567
### Security
6668

Plugins/SnowflakeDriverPlugin/SnowflakeConnection.swift

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,31 @@ final class SnowflakeConnection: @unchecked Sendable {
4040

4141
let host: String
4242
let params: ResolvedParameters
43+
private let connectionIdentifier: String
4344

4445
private let session: URLSession
4546
private let lock = NSLock()
4647
private let heartbeat = SnowflakeHeartbeat()
4748
private var sessionToken: String?
4849
private var renewalToken: String?
49-
private var activeRequestIDs: Set<String> = []
50+
private var activeRequestIDs: [String: Set<String>] = [:]
5051
private var sequenceId = 0
5152
private var connectTask: Task<Void, Error>?
5253

5354
var sessionFingerprint: String {
54-
[host, params.user.uppercased(), params.authMethod, params.role.uppercased()].joined(separator: "|")
55+
SnowflakeSessionKey.fingerprint(
56+
connectionId: connectionIdentifier,
57+
host: host,
58+
user: params.user,
59+
authMethod: params.authMethod,
60+
role: params.role
61+
)
5562
}
5663

64+
/// Statements the connection issues for itself, such as the connect-time `USE` calls, belong to
65+
/// no driver and are never the target of a Stop.
66+
static let sessionOwner = "session"
67+
5768
private var _currentDatabase: String?
5869
private var _currentSchema: String?
5970
private var _currentWarehouse: String?
@@ -71,6 +82,7 @@ final class SnowflakeConnection: @unchecked Sendable {
7182
init(config: DriverConnectionConfig) {
7283
self.params = Self.resolveParameters(from: config)
7384
self.host = SnowflakeAccount.host(forAccount: params.account)
85+
self.connectionIdentifier = config.additionalFields["connectionId"] ?? ""
7486

7587
let configuration = URLSessionConfiguration.ephemeral
7688
configuration.timeoutIntervalForRequest = 120
@@ -419,9 +431,13 @@ final class SnowflakeConnection: @unchecked Sendable {
419431

420432
// MARK: - Query Execution
421433

422-
func query(_ sql: String, parameters: [PluginCellValue] = []) async throws -> SnowflakeQueryResult {
434+
func query(
435+
_ sql: String,
436+
parameters: [PluginCellValue] = [],
437+
owner: String = SnowflakeConnection.sessionOwner
438+
) async throws -> SnowflakeQueryResult {
423439
try await withReauthentication {
424-
try await performQuery(sql, parameters: parameters)
440+
try await performQuery(sql, parameters: parameters, owner: owner)
425441
}
426442
}
427443

@@ -439,8 +455,12 @@ final class SnowflakeConnection: @unchecked Sendable {
439455
}
440456
}
441457

442-
func cancelAllQueries() {
443-
let (requestIDs, token) = lock.withLock { (activeRequestIDs, sessionToken) }
458+
/// One Snowflake session is shared by the driver the user sees and by every pooled metadata
459+
/// driver behind it, so an abort has to name whose work it is stopping. Aborting the whole
460+
/// session made Stop on a query cancel a sidebar refresh running beside it, and cancel a save's
461+
/// remaining statements.
462+
func cancelQueries(owner: String) {
463+
let (requestIDs, token) = lock.withLock { (activeRequestIDs[owner] ?? [], sessionToken) }
444464
guard !requestIDs.isEmpty, let token else { return }
445465
Task { [weak self] in
446466
for requestID in requestIDs {
@@ -454,8 +474,12 @@ final class SnowflakeConnection: @unchecked Sendable {
454474
}
455475
}
456476

457-
private func performQuery(_ sql: String, parameters: [PluginCellValue] = []) async throws -> SnowflakeQueryResult {
458-
let (data, token) = try await submitQuery(sql, parameters: parameters)
477+
private func performQuery(
478+
_ sql: String,
479+
parameters: [PluginCellValue] = [],
480+
owner: String
481+
) async throws -> SnowflakeQueryResult {
482+
let (data, token) = try await submitQuery(sql, parameters: parameters, owner: owner)
459483
if let resultIds = data["resultIds"] as? String, !resultIds.isEmpty {
460484
return try await collectMultiStatementResults(ids: resultIds, token: token)
461485
}
@@ -465,7 +489,8 @@ final class SnowflakeConnection: @unchecked Sendable {
465489

466490
private func submitQuery(
467491
_ sql: String,
468-
parameters: [PluginCellValue]
492+
parameters: [PluginCellValue],
493+
owner: String
469494
) async throws -> (data: [String: Any], token: String) {
470495
guard let token = lock.withLock({ sessionToken }) else {
471496
throw SnowflakeError.notConnected
@@ -474,11 +499,14 @@ final class SnowflakeConnection: @unchecked Sendable {
474499
let requestID = UUID().uuidString.lowercased()
475500
let sequence = lock.withLock { () -> Int in
476501
sequenceId += 1
477-
activeRequestIDs.insert(requestID)
502+
activeRequestIDs[owner, default: []].insert(requestID)
478503
return sequenceId
479504
}
480505
defer {
481-
lock.withLock { _ = activeRequestIDs.remove(requestID) }
506+
lock.withLock {
507+
activeRequestIDs[owner]?.remove(requestID)
508+
if activeRequestIDs[owner]?.isEmpty == true { activeRequestIDs[owner] = nil }
509+
}
482510
}
483511

484512
var body: [String: Any] = [
@@ -611,9 +639,12 @@ final class SnowflakeConnection: @unchecked Sendable {
611639
let batches: AsyncThrowingStream<[[PluginCellValueBox]], Error>
612640
}
613641

614-
func queryStreamed(_ sql: String) async throws -> StreamedResult {
642+
func queryStreamed(
643+
_ sql: String,
644+
owner: String = SnowflakeConnection.sessionOwner
645+
) async throws -> StreamedResult {
615646
let (data, _) = try await withReauthentication {
616-
try await submitQuery(sql, parameters: [])
647+
try await submitQuery(sql, parameters: [], owner: owner)
617648
}
618649
applyFinalSessionInfo(data)
619650

Plugins/SnowflakeDriverPlugin/SnowflakePluginDriver.swift

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ final class SnowflakePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
1717
private var resolvedSchemaCache: [String: String] = [:]
1818
private var columnTypeCache: [String: [String: String]] = [:]
1919

20+
/// Several drivers share one Snowflake session, so a Stop has to name its own work. This
21+
/// identifies the statements this driver issued and nobody else's.
22+
private let queryOwner = UUID().uuidString
23+
2024
private static let logger = Logger(subsystem: "com.TablePro", category: "SnowflakePluginDriver")
2125

2226
private var connection: SnowflakeConnection? {
@@ -41,7 +45,7 @@ final class SnowflakePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
4145
}
4246

4347
func cancelQuery() throws {
44-
connection?.cancelAllQueries()
48+
connection?.cancelQueries(owner: queryOwner)
4549
}
4650

4751
var supportsSchemas: Bool { true }
@@ -55,7 +59,7 @@ final class SnowflakePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
5559
guard !parameters.isEmpty else { return try await execute(query: query) }
5660
guard let conn = connection else { throw SnowflakeError.notConnected }
5761
let startTime = Date()
58-
let result = try await conn.query(query, parameters: parameters)
62+
let result = try await conn.query(query, parameters: parameters, owner: queryOwner)
5963
return PluginQueryResult(
6064
columns: result.columns.map(\.name),
6165
columnTypeNames: result.columns.map(SnowflakeTypeMapper.displayType),
@@ -79,7 +83,7 @@ final class SnowflakePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
7983
}
8084
lock.withLock { _connection = conn }
8185

82-
if let result = try? await conn.query("SELECT CURRENT_VERSION()"),
86+
if let result = try? await conn.query("SELECT CURRENT_VERSION()", owner: queryOwner),
8387
let first = result.rows.first?.first, case .text(let version) = first {
8488
lock.withLock { _serverVersion = "Snowflake \(version)" }
8589
} else {
@@ -114,7 +118,7 @@ final class SnowflakePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
114118
func execute(query: String) async throws -> PluginQueryResult {
115119
guard let conn = connection else { throw SnowflakeError.notConnected }
116120
let startTime = Date()
117-
let result = try await conn.query(query)
121+
let result = try await conn.query(query, owner: queryOwner)
118122
let executionTime = Date().timeIntervalSince(startTime)
119123

120124
if result.columns.isEmpty {
@@ -201,9 +205,9 @@ final class SnowflakePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
201205
guard let conn = connection else { throw SnowflakeError.notConnected }
202206
switch id {
203207
case "warehouse":
204-
_ = try await conn.query("USE WAREHOUSE \(quoteIdentifier(value))")
208+
_ = try await conn.query("USE WAREHOUSE \(quoteIdentifier(value))", owner: queryOwner)
205209
case "role":
206-
_ = try await conn.query("USE ROLE \(quoteIdentifier(value))")
210+
_ = try await conn.query("USE ROLE \(quoteIdentifier(value))", owner: queryOwner)
207211
lock.withLock {
208212
resolvedSchemaCache.removeAll()
209213
columnTypeCache.removeAll()
@@ -583,7 +587,7 @@ final class SnowflakePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
583587
do {
584588
guard let conn = self.connection else { throw SnowflakeError.notConnected }
585589
let trimmed = query.replacingOccurrences(of: ";\\s*\\z", with: "", options: .regularExpression)
586-
let streamed = try await conn.queryStreamed(trimmed)
590+
let streamed = try await conn.queryStreamed(trimmed, owner: queryOwner)
587591
continuation.yield(.header(PluginStreamHeader(
588592
columns: streamed.columns.map(\.name),
589593
columnTypeNames: streamed.columns.map(SnowflakeTypeMapper.displayType),
@@ -682,7 +686,7 @@ final class SnowflakePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
682686

683687
private func rawQuery(_ sql: String) async throws -> SnowflakeQueryResult {
684688
guard let conn = connection else { throw SnowflakeError.notConnected }
685-
return try await conn.query(sql)
689+
return try await conn.query(sql, owner: queryOwner)
686690
}
687691

688692
private func namedValues(in result: SnowflakeQueryResult, column: String) -> [String] {
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
//
2+
// SnowflakeSessionKey.swift
3+
// SnowflakeDriverPlugin
4+
//
5+
// Decides which drivers share one authenticated Snowflake session.
6+
//
7+
8+
import Foundation
9+
10+
enum SnowflakeSessionKey {
11+
/// Two saved connections can reach the same account, user and role and still be different
12+
/// connections. Keying on the account alone let them share a session, so `USE DATABASE` in one
13+
/// window moved the other window's current database.
14+
///
15+
/// The saved connection's own identifier separates them. The database cannot: the metadata pool
16+
/// rewrites it on its copy of the connection before building a driver, so including it here
17+
/// would give that driver a different key, a second login, and another MFA prompt, which is the
18+
/// whole reason the session is shared in the first place.
19+
static func fingerprint(
20+
connectionId: String,
21+
host: String,
22+
user: String,
23+
authMethod: String,
24+
role: String
25+
) -> String {
26+
[connectionId, host, user.uppercased(), authMethod, role.uppercased()].joined(separator: "|")
27+
}
28+
}

TablePro/Core/Database/DatabaseDriver.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,7 @@ enum DatabaseDriverFactory {
581581
additionalFields["enableCleartextPlugin"] = "true"
582582
}
583583
additionalFields["queryTimeoutSeconds"] = String(AppSettingsManager.shared.general.queryTimeoutSeconds)
584+
additionalFields["connectionId"] = connection.id.uuidString
584585
let config = DriverConnectionConfig(
585586
host: connection.host,
586587
port: connection.port,
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
//
2+
// SnowflakeSessionKeyTests.swift
3+
// TableProTests
4+
//
5+
// Tests for SnowflakeSessionKey (compiled via symlink from SnowflakeDriverPlugin).
6+
//
7+
8+
import Foundation
9+
import Testing
10+
11+
@Suite("Snowflake Session Key")
12+
struct SnowflakeSessionKeyTests {
13+
private func key(
14+
connectionId: String = "A",
15+
host: String = "acct.snowflakecomputing.com",
16+
user: String = "dana",
17+
authMethod: String = "password",
18+
role: String = "analyst"
19+
) -> String {
20+
SnowflakeSessionKey.fingerprint(
21+
connectionId: connectionId,
22+
host: host,
23+
user: user,
24+
authMethod: authMethod,
25+
role: role
26+
)
27+
}
28+
29+
/// The defect: two saved connections to one account, user and role shared a session, so
30+
/// `USE DATABASE` in one moved the other's current database and a save wrote to the wrong one.
31+
@Test("Two saved connections to the same account do not share a session")
32+
func testDistinctConnectionsDoNotShare() {
33+
#expect(key(connectionId: "A") != key(connectionId: "B"))
34+
}
35+
36+
@Test("The same saved connection always resolves to one session")
37+
func testSameConnectionShares() {
38+
#expect(key(connectionId: "A") == key(connectionId: "A"))
39+
}
40+
41+
/// The metadata pool rewrites the database on its copy before building a driver, so a key that
42+
/// varied with the database would give that driver its own login and another MFA prompt.
43+
@Test("The database is not part of the key")
44+
func testDatabaseIsNotInTheKey() {
45+
#expect(!key().contains("ANALYTICS"))
46+
#expect(key(connectionId: "A") == key(connectionId: "A"))
47+
}
48+
49+
@Test("Account identity still separates sessions")
50+
func testAccountIdentitySeparates() {
51+
#expect(key(host: "one.snowflakecomputing.com") != key(host: "two.snowflakecomputing.com"))
52+
#expect(key(user: "dana") != key(user: "sam"))
53+
#expect(key(authMethod: "password") != key(authMethod: "keyPair"))
54+
#expect(key(role: "analyst") != key(role: "admin"))
55+
}
56+
57+
@Test("User and role compare case-insensitively")
58+
func testCaseFolding() {
59+
#expect(key(user: "dana") == key(user: "DANA"))
60+
#expect(key(role: "analyst") == key(role: "ANALYST"))
61+
}
62+
}

project.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,7 @@ targets:
486486
- Plugins/SnowflakeDriverPlugin/SnowflakeMFATokenStore.swift
487487
- Plugins/SnowflakeDriverPlugin/SnowflakeObjectQueries.swift
488488
- Plugins/SnowflakeDriverPlugin/SnowflakeSchemaQueries.swift
489+
- Plugins/SnowflakeDriverPlugin/SnowflakeSessionKey.swift
489490
- Plugins/SnowflakeDriverPlugin/SnowflakeSQL.swift
490491
- Plugins/SnowflakeDriverPlugin/SnowflakeStatementGenerator.swift
491492
- Plugins/SnowflakeDriverPlugin/SnowflakeStatementType.swift

0 commit comments

Comments
 (0)