Skip to content

Commit 3f6a45f

Browse files
datlechindatlechin
committed
fix: critical review issues — semicolon regression, keyword scope SQL, transaction safety, sheet identity
1 parent f4a25eb commit 3f6a45f

7 files changed

Lines changed: 114 additions & 62 deletions

File tree

‎TablePro/Core/Storage/SQLFavoriteStorage.swift‎

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -534,15 +534,17 @@ internal final class SQLFavoriteStorage {
534534
return await performDatabaseWork { [weak self] in
535535
guard let self = self else { return false }
536536

537-
let inTransaction = sqlite3_exec(self.db, "BEGIN IMMEDIATE;", nil, nil, nil) == SQLITE_OK
537+
guard sqlite3_exec(self.db, "BEGIN IMMEDIATE;", nil, nil, nil) == SQLITE_OK else {
538+
return false
539+
}
538540

539541
let SQLITE_TRANSIENT = unsafeBitCast(-1, to: sqlite3_destructor_type.self)
540542

541543
// Find the parent_id of the folder being deleted
542544
let findParentSQL = "SELECT parent_id FROM folders WHERE id = ?;"
543545
var findStatement: OpaquePointer?
544546
guard sqlite3_prepare_v2(self.db, findParentSQL, -1, &findStatement, nil) == SQLITE_OK else {
545-
if inTransaction { sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil) }
547+
sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil)
546548
return false
547549
}
548550

@@ -567,12 +569,12 @@ internal final class SQLFavoriteStorage {
567569
let moveFavResult = sqlite3_step(moveFavStatement)
568570
sqlite3_finalize(moveFavStatement)
569571
if moveFavResult != SQLITE_DONE {
570-
if inTransaction { sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil) }
572+
sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil)
571573
return false
572574
}
573575
} else {
574576
sqlite3_finalize(moveFavStatement)
575-
if inTransaction { sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil) }
577+
sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil)
576578
return false
577579
}
578580

@@ -589,33 +591,31 @@ internal final class SQLFavoriteStorage {
589591
let moveSubResult = sqlite3_step(moveSubStatement)
590592
sqlite3_finalize(moveSubStatement)
591593
if moveSubResult != SQLITE_DONE {
592-
if inTransaction { sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil) }
594+
sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil)
593595
return false
594596
}
595597
} else {
596598
sqlite3_finalize(moveSubStatement)
597-
if inTransaction { sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil) }
599+
sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil)
598600
return false
599601
}
600602

601603
// Delete the folder
602604
let deleteSQL = "DELETE FROM folders WHERE id = ?;"
603605
var deleteStatement: OpaquePointer?
604606
guard sqlite3_prepare_v2(self.db, deleteSQL, -1, &deleteStatement, nil) == SQLITE_OK else {
605-
if inTransaction { sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil) }
607+
sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil)
606608
return false
607609
}
608610

609611
sqlite3_bind_text(deleteStatement, 1, idString, -1, SQLITE_TRANSIENT)
610612
let result = sqlite3_step(deleteStatement)
611613
sqlite3_finalize(deleteStatement)
612614

613-
if inTransaction {
614-
if result == SQLITE_DONE {
615-
sqlite3_exec(self.db, "COMMIT;", nil, nil, nil)
616-
} else {
617-
sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil)
618-
}
615+
if result == SQLITE_DONE {
616+
sqlite3_exec(self.db, "COMMIT;", nil, nil, nil)
617+
} else {
618+
sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil)
619619
}
620620

621621
return result == SQLITE_DONE
@@ -719,11 +719,22 @@ internal final class SQLFavoriteStorage {
719719
return await performDatabaseWork { [weak self] in
720720
guard let self = self else { return false }
721721

722-
var sql = """
723-
SELECT COUNT(*) FROM favorites
724-
WHERE keyword = ?
725-
AND (connection_id IS NULL OR connection_id = ? OR ? IS NULL)
726-
"""
722+
var sql: String
723+
var bindIndex: Int32 = 1
724+
725+
if connectionIdString != nil {
726+
sql = """
727+
SELECT COUNT(*) FROM favorites
728+
WHERE keyword = ?
729+
AND (connection_id IS NULL OR connection_id = ?)
730+
"""
731+
} else {
732+
sql = """
733+
SELECT COUNT(*) FROM favorites
734+
WHERE keyword = ?
735+
AND connection_id IS NULL
736+
"""
737+
}
727738

728739
if excludeIdString != nil {
729740
sql += " AND id != ?"
@@ -740,18 +751,16 @@ internal final class SQLFavoriteStorage {
740751

741752
let SQLITE_TRANSIENT = unsafeBitCast(-1, to: sqlite3_destructor_type.self)
742753

743-
sqlite3_bind_text(statement, 1, keyword, -1, SQLITE_TRANSIENT)
754+
sqlite3_bind_text(statement, bindIndex, keyword, -1, SQLITE_TRANSIENT)
755+
bindIndex += 1
744756

745757
if let connId = connectionIdString {
746-
sqlite3_bind_text(statement, 2, connId, -1, SQLITE_TRANSIENT)
747-
sqlite3_bind_text(statement, 3, connId, -1, SQLITE_TRANSIENT)
748-
} else {
749-
sqlite3_bind_null(statement, 2)
750-
sqlite3_bind_null(statement, 3)
758+
sqlite3_bind_text(statement, bindIndex, connId, -1, SQLITE_TRANSIENT)
759+
bindIndex += 1
751760
}
752761

753762
if let excludeId = excludeIdString {
754-
sqlite3_bind_text(statement, 4, excludeId, -1, SQLITE_TRANSIENT)
763+
sqlite3_bind_text(statement, bindIndex, excludeId, -1, SQLITE_TRANSIENT)
755764
}
756765

757766
if sqlite3_step(statement) == SQLITE_ROW {

‎TablePro/Core/Utilities/SQL/SQLStatementScanner.swift‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,28 @@ enum SQLStatementScanner {
1111
let offset: Int
1212
}
1313

14+
/// Returns statements with trailing semicolons stripped — for driver execution.
1415
static func allStatements(in sql: String) -> [String] {
16+
var results: [String] = []
17+
scan(sql: sql, cursorPosition: nil) { rawSQL, _ in
18+
var trimmed = rawSQL.trimmingCharacters(in: .whitespacesAndNewlines)
19+
if trimmed.hasSuffix(";") {
20+
trimmed = String(trimmed.dropLast())
21+
.trimmingCharacters(in: .whitespacesAndNewlines)
22+
}
23+
if !trimmed.isEmpty {
24+
results.append(trimmed)
25+
}
26+
return true
27+
}
28+
return results
29+
}
30+
31+
/// Returns statements preserving trailing semicolons — for display/history/favorites.
32+
static func allStatementsPreservingSemicolons(in sql: String) -> [String] {
1533
var results: [String] = []
1634
scan(sql: sql, cursorPosition: nil) { rawSQL, _ in
1735
let trimmed = rawSQL.trimmingCharacters(in: .whitespacesAndNewlines)
18-
// Skip empty statements (bare semicolons, whitespace-only)
1936
let withoutSemicolon = trimmed.hasSuffix(";")
2037
? String(trimmed.dropLast()).trimmingCharacters(in: .whitespacesAndNewlines)
2138
: trimmed
@@ -28,9 +45,14 @@ enum SQLStatementScanner {
2845
}
2946

3047
static func statementAtCursor(in sql: String, cursorPosition: Int) -> String {
31-
locatedStatementAtCursor(in: sql, cursorPosition: cursorPosition)
48+
var result = locatedStatementAtCursor(in: sql, cursorPosition: cursorPosition)
3249
.sql
3350
.trimmingCharacters(in: .whitespacesAndNewlines)
51+
if result.hasSuffix(";") {
52+
result = String(result.dropLast())
53+
.trimmingCharacters(in: .whitespacesAndNewlines)
54+
}
55+
return result
3456
}
3557

3658
static func locatedStatementAtCursor(in sql: String, cursorPosition: Int) -> LocatedStatement {

‎TablePro/ViewModels/FavoritesSidebarViewModel.swift‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@
66
import Foundation
77
import Observation
88

9+
/// Identity wrapper for presenting the favorite edit dialog via `.sheet(item:)`
10+
internal struct FavoriteEditItem: Identifiable {
11+
let id = UUID()
12+
let favorite: SQLFavorite?
13+
let query: String?
14+
let folderId: UUID?
15+
}
16+
917
/// Tree node for displaying favorites and folders in a hierarchy
1018
internal enum FavoriteTreeItem: Identifiable, Hashable {
1119
case folder(SQLFavoriteFolder, children: [FavoriteTreeItem])
@@ -26,7 +34,7 @@ internal final class FavoritesSidebarViewModel {
2634

2735
var treeItems: [FavoriteTreeItem] = []
2836
var isLoading = false
29-
var showEditDialog = false
37+
var editDialogItem: FavoriteEditItem?
3038
var editingFavorite: SQLFavorite?
3139
var editingQuery: String?
3240
var editingFolderId: UUID?
@@ -110,16 +118,11 @@ internal final class FavoritesSidebarViewModel {
110118
if let folderId {
111119
expandedFolderIds.insert(folderId)
112120
}
113-
editingFavorite = nil
114-
editingQuery = query
115-
editingFolderId = folderId
116-
showEditDialog = true
121+
editDialogItem = FavoriteEditItem(favorite: nil, query: query, folderId: folderId)
117122
}
118123

119124
func editFavorite(_ favorite: SQLFavorite) {
120-
editingFavorite = favorite
121-
editingQuery = nil
122-
showEditDialog = true
125+
editDialogItem = FavoriteEditItem(favorite: favorite, query: nil, folderId: favorite.folderId)
123126
}
124127

125128
func deleteFavorite(_ favorite: SQLFavorite) {

‎TablePro/Views/Main/Extensions/MainContentCoordinator+MultiStatement.swift‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,11 @@ extension MainContentCoordinator {
7070
lastSelectSQL = sql
7171
}
7272

73-
// Record each statement individually in query history
73+
// Record with semicolon preserved for history/favorites
74+
let historySQL = sql.hasSuffix(";") ? sql : sql + ";"
7475
await MainActor.run {
7576
QueryHistoryManager.shared.recordQuery(
76-
query: sql,
77+
query: historySQL,
7778
connectionId: conn.id,
7879
databaseName: conn.database,
7980
executionTime: result.executionTime,
@@ -160,8 +161,8 @@ extension MainContentCoordinator {
160161
tabManager.tabs[idx] = errTab
161162
}
162163

163-
// Record only the failing statement in history
164-
let recordSQL = failedSQL ?? statements[min(executedCount, totalCount - 1)]
164+
let rawSQL = failedSQL ?? statements[min(executedCount, totalCount - 1)]
165+
let recordSQL = rawSQL.hasSuffix(";") ? rawSQL : rawSQL + ";"
165166
QueryHistoryManager.shared.recordQuery(
166167
query: recordSQL,
167168
connectionId: conn.id,

‎TablePro/Views/Main/Extensions/MainContentCoordinator+SaveChanges.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,9 @@ extension MainContentCoordinator {
200200

201201
let executionTime = Date().timeIntervalSince(statementStartTime)
202202

203+
let historySQL = statement.sql.trimmingCharacters(in: .whitespacesAndNewlines)
203204
QueryHistoryManager.shared.recordQuery(
204-
query: statement.sql.trimmingCharacters(in: .whitespacesAndNewlines),
205+
query: historySQL.hasSuffix(";") ? historySQL : historySQL + ";",
205206
connectionId: conn.id,
206207
databaseName: conn.database,
207208
executionTime: executionTime,

‎TablePro/Views/Sidebar/FavoritesTabView.swift‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ internal struct FavoritesTabView: View {
1616
@FocusState private var isRenameFocused: Bool
1717
let connectionId: UUID
1818
let searchText: String
19-
private weak var coordinator: MainContentCoordinator?
19+
private var coordinator: MainContentCoordinator?
2020

2121
init(connectionId: UUID, searchText: String, coordinator: MainContentCoordinator?) {
2222
self.connectionId = connectionId
@@ -46,12 +46,12 @@ internal struct FavoritesTabView: View {
4646
.onAppear {
4747
Task { await viewModel.loadFavorites() }
4848
}
49-
.sheet(isPresented: $viewModel.showEditDialog) {
49+
.sheet(item: $viewModel.editDialogItem) { item in
5050
FavoriteEditDialog(
5151
connectionId: connectionId,
52-
favorite: viewModel.editingFavorite,
53-
initialQuery: viewModel.editingQuery,
54-
folderId: viewModel.editingFolderId
52+
favorite: item.favorite,
53+
initialQuery: item.query,
54+
folderId: item.folderId
5555
)
5656
}
5757
.alert(
@@ -260,7 +260,7 @@ internal struct FavoritesTabView: View {
260260
private struct FavoriteItemContextMenu: View {
261261
let favorite: SQLFavorite
262262
let viewModel: FavoritesSidebarViewModel
263-
weak var coordinator: MainContentCoordinator?
263+
var coordinator: MainContentCoordinator?
264264

265265
private var folders: [SQLFavoriteFolder] {
266266
collectFolders(from: viewModel.treeItems)

0 commit comments

Comments
 (0)