Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
407 changes: 0 additions & 407 deletions SECURITY_AUDIT.md

This file was deleted.

13 changes: 7 additions & 6 deletions TablePro/Core/ChangeTracking/DataChangeManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -510,14 +510,15 @@ final class DataChangeManager: ObservableObject {
insertedRowIndices: insertedRowIndices
)

// Count expected statements (updates + deletes, inserts are separate)
let expectedUpdateDeletes = changes.filter { $0.type == .update || $0.type == .delete }.count
let actualStatements = statements.filter { !$0.contains("INSERT INTO") }.count
// Count expected UPDATE statements (DELETEs can work without PK using full row match)
let expectedUpdates = changes.filter { $0.type == .update }.count
let actualUpdates = statements.filter { $0.hasPrefix("UPDATE") }.count

// Check if any UPDATE/DELETE statements were skipped due to missing primary key
if expectedUpdateDeletes > 0 && actualStatements < expectedUpdateDeletes {
// Check if any UPDATE statements were skipped due to missing primary key
// Note: DELETEs are allowed without PK (they match all columns)
if expectedUpdates > 0 && actualUpdates < expectedUpdates {
throw DatabaseError.queryFailed(
"Cannot save changes to table '\(tableName)' without a primary key. " +
"Cannot save UPDATE changes to table '\(tableName)' without a primary key. " +
"Please add a primary key to this table or use raw SQL queries instead."
)
}
Expand Down
100 changes: 78 additions & 22 deletions TablePro/Core/ChangeTracking/SQLStatementGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,29 @@ struct SQLStatementGenerator {
}
}

// Generate batched UPDATE statements (group by same columns being updated)
// Generate individual UPDATE statements with LIMIT 1 (safer than batched CASE/WHEN)
// This prevents accidentally updating multiple rows with the same value
if !updateChanges.isEmpty {
let batchedUpdates = generateBatchUpdateSQL(for: updateChanges)
statements.append(contentsOf: batchedUpdates)
for change in updateChanges {
if let sql = generateUpdateSQL(for: change) {
statements.append(sql)
}
}
}

// Generate batched DELETE statement (single DELETE with OR conditions)
// Generate DELETE statements
// Try batched DELETE first (uses PK if available), fall back to individual DELETEs
if !deleteChanges.isEmpty {
if let sql = generateBatchDeleteSQL(for: deleteChanges) {
// Batched delete successful (has PK)
statements.append(sql)
} else {
// No PK - generate individual DELETE statements matching all columns
for change in deleteChanges {
if let sql = generateDeleteSQL(for: change) {
statements.append(sql)
}
}
}
}

Expand Down Expand Up @@ -277,7 +290,12 @@ struct SQLStatementGenerator {
}

let whereClause = "\(databaseType.quoteIdentifier(pkColumn)) = \(pkValue)"
return "UPDATE \(databaseType.quoteIdentifier(tableName)) SET \(setClauses) WHERE \(whereClause)"

// Add LIMIT 1 for MySQL/MariaDB to ensure only one row is updated (TablePlus-style safety)
// PostgreSQL doesn't support LIMIT in UPDATE, but the PK constraint ensures single row
let limitClause = (databaseType == .mysql || databaseType == .mariadb) ? " LIMIT 1" : ""

return "UPDATE \(databaseType.quoteIdentifier(tableName)) SET \(setClauses) WHERE \(whereClause)\(limitClause)"
}

// MARK: - DELETE Generation
Expand All @@ -286,27 +304,65 @@ struct SQLStatementGenerator {
/// Example: DELETE FROM table WHERE id = 1 OR id = 2 OR id = 3
private func generateBatchDeleteSQL(for changes: [RowChange]) -> String? {
guard !changes.isEmpty else { return nil }
guard let pkColumn = primaryKeyColumn else { return nil }
guard let pkIndex = columns.firstIndex(of: pkColumn) else { return nil }

// Build OR conditions for all rows

// If we have a primary key, use it for efficient deletion
if let pkColumn = primaryKeyColumn,
let pkIndex = columns.firstIndex(of: pkColumn) {

// Build OR conditions for all rows using PK
var conditions: [String] = []

for change in changes {
guard let originalRow = change.originalRow,
pkIndex < originalRow.count else {
continue
}

let pkValue = originalRow[pkIndex].map { "'\(escapeSQLString($0))'" } ?? "NULL"
conditions.append("\(databaseType.quoteIdentifier(pkColumn)) = \(pkValue)")
}

guard !conditions.isEmpty else { return nil }

// Combine all conditions with OR
let whereClause = conditions.joined(separator: " OR ")
return "DELETE FROM \(databaseType.quoteIdentifier(tableName)) WHERE \(whereClause)"
}

// Fallback: No primary key - generate individual DELETE statements matching all columns
// This is safe but requires exact row matching
return nil // Return nil to trigger individual DELETE generation
}

/// Generate individual DELETE statement for a single row (used when no PK or as fallback)
/// Matches all column values to ensure we delete the exact row
private func generateDeleteSQL(for change: RowChange) -> String? {
guard let originalRow = change.originalRow else { return nil }

// Build WHERE clause matching ALL columns to uniquely identify the row
var conditions: [String] = []

for change in changes {
guard let originalRow = change.originalRow,
pkIndex < originalRow.count else {
continue

for (index, columnName) in columns.enumerated() {
guard index < originalRow.count else { continue }

let value = originalRow[index]
let quotedColumn = databaseType.quoteIdentifier(columnName)

if let value = value {
conditions.append("\(quotedColumn) = '\(escapeSQLString(value))'")
} else {
conditions.append("\(quotedColumn) IS NULL")
}

let pkValue = originalRow[pkIndex].map { "'\(escapeSQLString($0))'" } ?? "NULL"
conditions.append("\(databaseType.quoteIdentifier(pkColumn)) = \(pkValue)")
}

guard !conditions.isEmpty else { return nil }

// Combine all conditions with OR
let whereClause = conditions.joined(separator: " OR ")
return "DELETE FROM \(databaseType.quoteIdentifier(tableName)) WHERE \(whereClause)"

let whereClause = conditions.joined(separator: " AND ")

// Add LIMIT 1 for MySQL/MariaDB to be extra safe
let limitClause = (databaseType == .mysql || databaseType == .mariadb) ? " LIMIT 1" : ""

return "DELETE FROM \(databaseType.quoteIdentifier(tableName)) WHERE \(whereClause)\(limitClause)"
}

// MARK: - Helper Functions
Expand Down
Loading