Skip to content

Commit 1e56672

Browse files
committed
fix: address review findings for DuckDB plugin driver
1 parent f1f6ea4 commit 1e56672

File tree

1 file changed

+69
-23
lines changed

1 file changed

+69
-23
lines changed

Plugins/DuckDBDriverPlugin/DuckDBPlugin.swift

Lines changed: 69 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -284,13 +284,17 @@ private struct DuckDBRawResult: Sendable {
284284
final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
285285
private let config: DriverConnectionConfig
286286
private let connectionActor = DuckDBConnectionActor()
287-
private let interruptLock = NSLock()
287+
private let stateLock = NSLock()
288288
nonisolated(unsafe) private var _connectionForInterrupt: duckdb_connection?
289-
private var _currentSchema: String = "main"
289+
nonisolated(unsafe) private var _currentSchema: String = "main"
290290

291291
private static let logger = Logger(subsystem: "com.TablePro", category: "DuckDBPluginDriver")
292292

293-
var currentSchema: String? { _currentSchema }
293+
var currentSchema: String? {
294+
stateLock.lock()
295+
defer { stateLock.unlock() }
296+
return _currentSchema
297+
}
294298
var serverVersion: String? { String(cString: duckdb_library_version()) }
295299
var supportsSchemas: Bool { true }
296300
var supportsTransactions: Bool { true }
@@ -299,6 +303,13 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
299303
self.config = config
300304
}
301305

306+
private func resolveSchema(_ schema: String?) -> String {
307+
if let schema { return schema }
308+
stateLock.lock()
309+
defer { stateLock.unlock() }
310+
return _currentSchema
311+
}
312+
302313
// MARK: - Connection
303314

304315
func connect() async throws {
@@ -326,9 +337,9 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
326337
}
327338

328339
func disconnect() {
329-
interruptLock.lock()
340+
stateLock.lock()
330341
_connectionForInterrupt = nil
331-
interruptLock.unlock()
342+
stateLock.unlock()
332343
let actor = connectionActor
333344
Task { await actor.close() }
334345
}
@@ -371,9 +382,9 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
371382
}
372383

373384
func cancelQuery() throws {
374-
interruptLock.lock()
385+
stateLock.lock()
375386
let conn = _connectionForInterrupt
376-
interruptLock.unlock()
387+
stateLock.unlock()
377388
guard let conn else { return }
378389
duckdb_interrupt(conn)
379390
}
@@ -397,7 +408,7 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
397408
// MARK: - Schema Operations
398409

399410
func fetchTables(schema: String?) async throws -> [PluginTableInfo] {
400-
let schemaName = schema ?? _currentSchema
411+
let schemaName = resolveSchema(schema)
401412
let query = """
402413
SELECT table_name, table_type
403414
FROM information_schema.tables
@@ -414,7 +425,7 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
414425
}
415426

416427
func fetchColumns(table: String, schema: String?) async throws -> [PluginColumnInfo] {
417-
let schemaName = schema ?? _currentSchema
428+
let schemaName = resolveSchema(schema)
418429
let query = """
419430
SELECT column_name, data_type, is_nullable, column_default, ordinal_position
420431
FROM information_schema.columns
@@ -447,7 +458,7 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
447458
}
448459

449460
func fetchAllColumns(schema: String?) async throws -> [String: [PluginColumnInfo]] {
450-
let schemaName = schema ?? _currentSchema
461+
let schemaName = resolveSchema(schema)
451462
let query = """
452463
SELECT table_name, column_name, data_type, is_nullable, column_default, ordinal_position
453464
FROM information_schema.columns
@@ -501,7 +512,7 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
501512
}
502513

503514
func fetchIndexes(table: String, schema: String?) async throws -> [PluginIndexInfo] {
504-
let schemaName = schema ?? _currentSchema
515+
let schemaName = resolveSchema(schema)
505516
let query = """
506517
SELECT index_name, is_unique, sql, index_oid
507518
FROM duckdb_indexes()
@@ -536,7 +547,7 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
536547
}
537548

538549
func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] {
539-
let schemaName = schema ?? _currentSchema
550+
let schemaName = resolveSchema(schema)
540551
let query = """
541552
SELECT
542553
rc.constraint_name,
@@ -587,7 +598,29 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
587598
}
588599

589600
func fetchTableDDL(table: String, schema: String?) async throws -> String {
590-
let schemaName = schema ?? _currentSchema
601+
let schemaName = resolveSchema(schema)
602+
603+
// Try native DDL from duckdb_tables() first (preserves complex types like LIST, STRUCT, MAP)
604+
let nativeQuery = "SELECT sql FROM duckdb_tables() WHERE schema_name = $1 AND table_name = $2"
605+
let nativeResult = try await executeParameterized(query: nativeQuery, parameters: [schemaName, table])
606+
607+
if let firstRow = nativeResult.rows.first, let sql = firstRow[0] {
608+
var ddl = sql.hasSuffix(";") ? sql : sql + ";"
609+
610+
// Append index definitions
611+
let indexes = try await fetchIndexes(table: table, schema: schemaName)
612+
for index in indexes where !index.isPrimary {
613+
let uniqueStr = index.isUnique ? "UNIQUE " : ""
614+
let cols = index.columns.map { "\"\(escapeIdentifier($0))\"" }.joined(separator: ", ")
615+
ddl += "\n\nCREATE \(uniqueStr)INDEX \"\(escapeIdentifier(index.name))\""
616+
+ " ON \"\(escapeIdentifier(schemaName))\".\"\(escapeIdentifier(table))\""
617+
+ " (\(cols));"
618+
}
619+
620+
return ddl
621+
}
622+
623+
// Fallback: synthesize DDL from schema metadata
591624
let columns = try await fetchColumns(table: table, schema: schemaName)
592625
let indexes = try await fetchIndexes(table: table, schema: schemaName)
593626
let fks = try await fetchForeignKeys(table: table, schema: schemaName)
@@ -633,7 +666,7 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
633666
}
634667

635668
func fetchViewDefinition(view: String, schema: String?) async throws -> String {
636-
let schemaName = schema ?? _currentSchema
669+
let schemaName = resolveSchema(schema)
637670
let query = """
638671
SELECT view_definition
639672
FROM information_schema.views
@@ -653,7 +686,7 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
653686
}
654687

655688
func fetchTableMetadata(table: String, schema: String?) async throws -> PluginTableMetadata {
656-
let schemaName = schema ?? _currentSchema
689+
let schemaName = resolveSchema(schema)
657690
let safeTable = escapeIdentifier(table)
658691
let safeSchema = escapeIdentifier(schemaName)
659692
let countQuery =
@@ -680,18 +713,20 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
680713
}
681714

682715
func switchSchema(to schema: String) async throws {
683-
let escaped = schema.replacingOccurrences(of: "'", with: "''")
684-
_ = try await execute(query: "SET schema = '\(escaped)'")
716+
let safeSchema = escapeIdentifier(schema)
717+
_ = try await execute(query: "SET schema = \"\(safeSchema)\"")
718+
stateLock.lock()
685719
_currentSchema = schema
720+
stateLock.unlock()
686721
}
687722

688723
// MARK: - Database Operations
689724

690725
func fetchDatabases() async throws -> [String] {
691-
let query = "PRAGMA database_list"
726+
let query = "SELECT database_name FROM duckdb_databases() ORDER BY database_name"
692727
let result = try await execute(query: query)
693728
return result.rows.compactMap { row in
694-
row[safe: 1] ?? nil
729+
row[safe: 0] ?? nil
695730
}
696731
}
697732

@@ -706,9 +741,9 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
706741
// MARK: - Private Helpers
707742

708743
nonisolated private func setInterruptHandle(_ handle: duckdb_connection?) {
709-
interruptLock.lock()
744+
stateLock.lock()
710745
_connectionForInterrupt = handle
711-
interruptLock.unlock()
746+
stateLock.unlock()
712747
}
713748

714749
private func expandPath(_ path: String) -> String {
@@ -752,26 +787,37 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
752787
let parenOpen = UInt16(UnicodeScalar("(").value)
753788
let parenClose = UInt16(UnicodeScalar(")").value)
754789
let singleQuote = UInt16(UnicodeScalar("'").value)
790+
let doubleQuote = UInt16(UnicodeScalar("\"").value)
755791

756792
var depth = 0
757793
var inString = false
794+
var inIdentifier = false
758795
var i = length - 1
759796

760797
while i >= 0 {
761798
let ch = upper.character(at: i)
762799

763800
if inString {
764801
if ch == singleQuote {
765-
// Check for escaped quote ('')
766802
if i > 0 && upper.character(at: i - 1) == singleQuote {
767-
i -= 1 // Skip escaped quote pair
803+
i -= 1
768804
} else {
769805
inString = false
770806
}
771807
}
808+
} else if inIdentifier {
809+
if ch == doubleQuote {
810+
if i > 0 && upper.character(at: i - 1) == doubleQuote {
811+
i -= 1
812+
} else {
813+
inIdentifier = false
814+
}
815+
}
772816
} else {
773817
if ch == singleQuote {
774818
inString = true
819+
} else if ch == doubleQuote {
820+
inIdentifier = true
775821
} else if ch == parenClose {
776822
depth += 1
777823
} else if ch == parenOpen {

0 commit comments

Comments
 (0)