Skip to content

Commit 932d762

Browse files
committed
fix: use DuckDB prepared statements and fix review issues
- Replace manual string substitution in executeParameterized with proper duckdb_prepare/duckdb_bind_varchar/duckdb_execute_prepared API calls - Convert all schema introspection queries to use parameterized queries ($1, $2 placeholders) instead of string interpolation - Add missing subquery alias in fetchTableMetadata count query - Fix extractIndexColumns to use regex matching ON clause instead of fragile backwards parenthesis search - Replace libduckdb.a duplicate with symlink to libduckdb_universal.a - Extract shared result parsing into extractResult helper method - Remove unused escapeStringLiteral method
1 parent 9572160 commit 932d762

2 files changed

Lines changed: 115 additions & 47 deletions

File tree

Libs/libduckdb.a

Lines changed: 0 additions & 3 deletions
This file was deleted.

Libs/libduckdb.a

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
libduckdb_universal.a

Plugins/DuckDBDriverPlugin/DuckDBPlugin.swift

Lines changed: 114 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,73 @@ private actor DuckDBConnectionActor {
9494
duckdb_destroy_result(&result)
9595
}
9696

97+
return Self.extractResult(from: &result, startTime: startTime)
98+
}
99+
100+
func executePrepared(_ query: String, parameters: [String?]) throws -> DuckDBRawResult {
101+
guard let conn = connection else {
102+
throw DuckDBPluginError.notConnected
103+
}
104+
105+
let startTime = Date()
106+
var stmt: duckdb_prepared_statement?
107+
108+
let prepState = duckdb_prepare(conn, query, &stmt)
109+
if prepState == DuckDBError {
110+
let errorMsg: String
111+
if let errPtr = duckdb_prepare_error(stmt) {
112+
errorMsg = String(cString: errPtr)
113+
} else {
114+
errorMsg = "Failed to prepare statement"
115+
}
116+
duckdb_destroy_prepare(&stmt)
117+
throw DuckDBPluginError.queryFailed(errorMsg)
118+
}
119+
120+
defer {
121+
duckdb_destroy_prepare(&stmt)
122+
}
123+
124+
for (index, param) in parameters.enumerated() {
125+
let paramIdx = idx_t(index + 1)
126+
if let value = param {
127+
let bindState = duckdb_bind_varchar(stmt, paramIdx, value)
128+
if bindState == DuckDBError {
129+
throw DuckDBPluginError.queryFailed("Failed to bind parameter at index \(index)")
130+
}
131+
} else {
132+
let bindState = duckdb_bind_null(stmt, paramIdx)
133+
if bindState == DuckDBError {
134+
throw DuckDBPluginError.queryFailed("Failed to bind NULL at index \(index)")
135+
}
136+
}
137+
}
138+
139+
var result = duckdb_result()
140+
let execState = duckdb_execute_prepared(stmt, &result)
141+
142+
if execState == DuckDBError {
143+
let errorMsg: String
144+
if let errPtr = duckdb_result_error(&result) {
145+
errorMsg = String(cString: errPtr)
146+
} else {
147+
errorMsg = "Failed to execute prepared statement"
148+
}
149+
duckdb_destroy_result(&result)
150+
throw DuckDBPluginError.queryFailed(errorMsg)
151+
}
152+
153+
defer {
154+
duckdb_destroy_result(&result)
155+
}
156+
157+
return Self.extractResult(from: &result, startTime: startTime)
158+
}
159+
160+
private static func extractResult(
161+
from result: inout duckdb_result,
162+
startTime: Date
163+
) -> DuckDBRawResult {
97164
let colCount = duckdb_column_count(&result)
98165
let rowCount = duckdb_row_count(&result)
99166
let rowsChanged = duckdb_rows_changed(&result)
@@ -272,18 +339,15 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
272339
query: String,
273340
parameters: [String?]
274341
) async throws -> PluginQueryResult {
275-
var processedQuery = query
276-
for param in parameters {
277-
if let range = processedQuery.range(of: "?") {
278-
if let value = param {
279-
let escaped = value.replacingOccurrences(of: "'", with: "''")
280-
processedQuery.replaceSubrange(range, with: "'\(escaped)'")
281-
} else {
282-
processedQuery.replaceSubrange(range, with: "NULL")
283-
}
284-
}
285-
}
286-
return try await execute(query: processedQuery)
342+
let rawResult = try await connectionActor.executePrepared(query, parameters: parameters)
343+
return PluginQueryResult(
344+
columns: rawResult.columns,
345+
columnTypeNames: rawResult.columnTypeNames,
346+
rows: rawResult.rows,
347+
rowsAffected: rawResult.rowsAffected,
348+
executionTime: rawResult.executionTime,
349+
isTruncated: rawResult.isTruncated
350+
)
287351
}
288352

289353
func cancelQuery() throws {
@@ -317,10 +381,10 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
317381
let query = """
318382
SELECT table_name, table_type
319383
FROM information_schema.tables
320-
WHERE table_schema = '\(escapeStringLiteral(schemaName))'
384+
WHERE table_schema = $1
321385
ORDER BY table_name
322386
"""
323-
let result = try await execute(query: query)
387+
let result = try await executeParameterized(query: query, parameters: [schemaName])
324388
return result.rows.compactMap { row in
325389
guard let name = row[safe: 0] ?? nil else { return nil }
326390
let typeString = (row[safe: 1] ?? nil) ?? "BASE TABLE"
@@ -334,11 +398,11 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
334398
let query = """
335399
SELECT column_name, data_type, is_nullable, column_default, ordinal_position
336400
FROM information_schema.columns
337-
WHERE table_schema = '\(escapeStringLiteral(schemaName))'
338-
AND table_name = '\(escapeStringLiteral(table))'
401+
WHERE table_schema = $1
402+
AND table_name = $2
339403
ORDER BY ordinal_position
340404
"""
341-
let result = try await execute(query: query)
405+
let result = try await executeParameterized(query: query, parameters: [schemaName, table])
342406

343407
let pkColumns = try await fetchPrimaryKeyColumns(table: table, schema: schemaName)
344408

@@ -367,10 +431,10 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
367431
let query = """
368432
SELECT table_name, column_name, data_type, is_nullable, column_default, ordinal_position
369433
FROM information_schema.columns
370-
WHERE table_schema = '\(escapeStringLiteral(schemaName))'
434+
WHERE table_schema = $1
371435
ORDER BY table_name, ordinal_position
372436
"""
373-
let result = try await execute(query: query)
437+
let result = try await executeParameterized(query: query, parameters: [schemaName])
374438

375439
let pkQuery = """
376440
SELECT tc.table_name, kcu.column_name
@@ -379,9 +443,9 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
379443
ON tc.constraint_name = kcu.constraint_name
380444
AND tc.table_schema = kcu.table_schema
381445
WHERE tc.constraint_type = 'PRIMARY KEY'
382-
AND tc.table_schema = '\(escapeStringLiteral(schemaName))'
446+
AND tc.table_schema = $1
383447
"""
384-
let pkResult = try await execute(query: pkQuery)
448+
let pkResult = try await executeParameterized(query: pkQuery, parameters: [schemaName])
385449
var pkMap: [String: Set<String>] = [:]
386450
for row in pkResult.rows {
387451
if let tableName = row[safe: 0] ?? nil, let colName = row[safe: 1] ?? nil {
@@ -421,12 +485,14 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
421485
let query = """
422486
SELECT index_name, is_unique, sql, index_oid
423487
FROM duckdb_indexes()
424-
WHERE schema_name = '\(escapeStringLiteral(schemaName))'
425-
AND table_name = '\(escapeStringLiteral(table))'
488+
WHERE schema_name = $1
489+
AND table_name = $2
426490
"""
427491

428492
do {
429-
let result = try await execute(query: query)
493+
let result = try await executeParameterized(
494+
query: query, parameters: [schemaName, table]
495+
)
430496
return result.rows.compactMap { row in
431497
guard let name = row[safe: 0] ?? nil else { return nil }
432498
let isUnique = (row[safe: 1] ?? nil) == "true"
@@ -467,12 +533,14 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
467533
ON rc.unique_constraint_name = kcu2.constraint_name
468534
AND rc.unique_constraint_schema = kcu2.constraint_schema
469535
AND kcu.ordinal_position = kcu2.ordinal_position
470-
WHERE kcu.table_schema = '\(escapeStringLiteral(schemaName))'
471-
AND kcu.table_name = '\(escapeStringLiteral(table))'
536+
WHERE kcu.table_schema = $1
537+
AND kcu.table_name = $2
472538
"""
473539

474540
do {
475-
let result = try await execute(query: query)
541+
let result = try await executeParameterized(
542+
query: query, parameters: [schemaName, table]
543+
)
476544
return result.rows.compactMap { row in
477545
guard let name = row[safe: 0] ?? nil,
478546
let column = row[safe: 1] ?? nil,
@@ -549,10 +617,10 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
549617
let query = """
550618
SELECT view_definition
551619
FROM information_schema.views
552-
WHERE table_schema = '\(escapeStringLiteral(schemaName))'
553-
AND table_name = '\(escapeStringLiteral(view))'
620+
WHERE table_schema = $1
621+
AND table_name = $2
554622
"""
555-
let result = try await execute(query: query)
623+
let result = try await executeParameterized(query: query, parameters: [schemaName, view])
556624

557625
guard let firstRow = result.rows.first,
558626
let definition = firstRow[0] else {
@@ -569,7 +637,7 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
569637
let safeTable = escapeIdentifier(table)
570638
let safeSchema = escapeIdentifier(schemaName)
571639
let countQuery =
572-
"SELECT COUNT(*) FROM (SELECT 1 FROM \"\(safeSchema)\".\"\(safeTable)\" LIMIT 100001)"
640+
"SELECT COUNT(*) FROM (SELECT 1 FROM \"\(safeSchema)\".\"\(safeTable)\" LIMIT 100001) AS _t"
573641
let countResult = try await execute(query: countQuery)
574642
let rowCount: Int64? = {
575643
guard let row = countResult.rows.first, let countStr = row.first else { return nil }
@@ -592,7 +660,8 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
592660
}
593661

594662
func switchSchema(to schema: String) async throws {
595-
_ = try await execute(query: "SET schema = '\(escapeStringLiteral(schema))'")
663+
let escaped = schema.replacingOccurrences(of: "'", with: "''")
664+
_ = try await execute(query: "SET schema = '\(escaped)'")
596665
_currentSchema = schema
597666
}
598667

@@ -629,10 +698,6 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
629698
return path
630699
}
631700

632-
private func escapeStringLiteral(_ value: String) -> String {
633-
value.replacingOccurrences(of: "'", with: "''")
634-
}
635-
636701
private func escapeIdentifier(_ value: String) -> String {
637702
value.replacingOccurrences(of: "\"", with: "\"\"")
638703
}
@@ -668,23 +733,28 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
668733
ON tc.constraint_name = kcu.constraint_name
669734
AND tc.table_schema = kcu.table_schema
670735
WHERE tc.constraint_type = 'PRIMARY KEY'
671-
AND tc.table_schema = '\(escapeStringLiteral(schema))'
672-
AND tc.table_name = '\(escapeStringLiteral(table))'
736+
AND tc.table_schema = $1
737+
AND tc.table_name = $2
673738
"""
674-
let result = try await execute(query: query)
739+
let result = try await executeParameterized(query: query, parameters: [schema, table])
675740
return Set(result.rows.compactMap { $0[safe: 0] ?? nil })
676741
}
677742

743+
private static let indexColumnsRegex = try? NSRegularExpression(
744+
pattern: #"ON\s+(?:"[^"]*"|[^\s(]+)\s*\(([^)]+)\)"#, options: .caseInsensitive
745+
)
746+
678747
private func extractIndexColumns(from sql: String?) -> [String] {
679-
guard let sql else { return [] }
748+
guard let sql, let regex = Self.indexColumnsRegex else { return [] }
680749

681-
guard let parenRange = sql.range(of: "(", options: .backwards),
682-
let closeRange = sql.range(of: ")", options: .backwards) else {
750+
let range = NSRange(sql.startIndex..., in: sql)
751+
guard let match = regex.firstMatch(in: sql, range: range),
752+
match.numberOfRanges > 1,
753+
let columnsRange = Range(match.range(at: 1), in: sql) else {
683754
return []
684755
}
685756

686-
let columnsStr = String(sql[parenRange.upperBound..<closeRange.lowerBound])
687-
return columnsStr.split(separator: ",").map {
757+
return String(sql[columnsRange]).split(separator: ",").map {
688758
$0.trimmingCharacters(in: .whitespaces)
689759
.replacingOccurrences(of: "\"", with: "")
690760
}

0 commit comments

Comments
 (0)