fix(clickhouse): refuse grid edits and deletes that reach other rows (#3794)

Author: datlechin

TablePro/Core/DataWrite/RowChangeStatementFactory.swift

@@ -124,6 +124,7 @@ struct RowChangeStatementFactory {
         ) {
             return .driverWritten(driverStatements)
         }
+        try refuseMatchesReachingOtherRows(in: changes, deletedRowIDs: deletedRowIDs)
         return .counted(
             try hostStatements(
                 for: changes,
@@ -134,6 +135,44 @@ struct RowChangeStatementFactory {
         )
     }
 
+    // These engines write their own rows, yet their saves reach the host's UPDATE and DELETE, keyed on a key the
+    // engine does not keep unique, with no row count to hold them to (#3776). Dameng enforces its primary key.
+    private static let enginesWithoutUniqueRowMatch: Set<DatabaseType> = [.clickhouse, .snowflake, .trino]
+
+    private func refuseMatchesReachingOtherRows(in changes: [RowChange], deletedRowIDs: Set<RowID>) throws {
+        let reason: String
+        if Self.enginesWithoutUniqueRowMatch.contains(databaseType) {
+            reason = String(
+                format: String(
+                    localized: "This version of TablePro cannot write %@ rows from the grid. Use a SQL statement instead."
+                ),
+                databaseType.displayName
+            )
+        } else if databaseType == .dameng, primaryKeyColumns.isEmpty {
+            reason = String(
+                format: String(
+                    localized: """
+                    This version of TablePro cannot write rows of a %@ table without a primary key from the grid. \
+                    Use a SQL statement instead.
+                    """
+                ),
+                databaseType.displayName
+            )
+        } else {
+            return
+        }
+        for change in changes {
+            switch change.type {
+            case .update:
+                throw DataWriteError.changeRefused(table: tableName, kind: .update, reason: reason)
+            case .delete where deletedRowIDs.contains(change.rowID):
+                throw DataWriteError.changeRefused(table: tableName, kind: .delete, reason: reason)
+            case .insert, .delete:
+                continue
+            }
+        }
+    }
+
     /// A value staged for a column the server owns can never be written, however it got staged: the grid offers no
     /// editor for one, but the set of such columns is empty until the table's schema arrives, and an edit made in
     /// that window survives its arrival. Sending it fails on the server (SQL Server Msg 8102 for an `IDENTITY`), and a

TableProTests/Core/ChangeTracking/DataChangeManagerClickHouseTests.swift

@@ -2,9 +2,6 @@
 //  DataChangeManagerClickHouseTests.swift
 //  TableProTests
 //
-//  Tests for ClickHouse-specific UPDATE statement validation in DataChangeManager.
-//  ClickHouse uses ALTER TABLE ... UPDATE syntax instead of standard UPDATE.
-//
 
 import Foundation
 import TableProPluginKit
@@ -13,8 +10,8 @@ import Testing
 
 @MainActor
 struct DataChangeManagerClickHouseTests {
-    @Test("ClickHouse ALTER TABLE UPDATE passes validation without throwing")
-    func alterTableUpdatePassesValidation() async {
+    @Test("A ClickHouse edit is refused and stays pending instead of keyed on the sorting key")
+    func clickHouseEditIsRefusedAndStaysPending() async {
         let manager = DataChangeManager()
         manager.configureForTable(
             tableName: "events",
@@ -33,10 +30,10 @@ struct DataChangeManagerClickHouseTests {
             originalRow: ["42", "pending"]
         )
 
-        // Should not throw — ALTER TABLE UPDATE must be recognized as valid
-        #expect(throws: Never.self) {
+        #expect(throws: DataWriteError.self) {
             _ = try manager.generateSQL()
         }
+        #expect(manager.hasChanges)
     }
 
     @Test("Standard UPDATE prefix is still detected for non-ClickHouse databases")

TableProTests/Core/DataWrite/RowChangeStatementFactoryUniqueMatchTests.swift

@@ -0,0 +1,140 @@
+//
+//  RowChangeStatementFactoryUniqueMatchTests.swift
+//  TableProTests
+//
+
+import Foundation
+@testable import TablePro
+import TableProPluginKit
+import Testing
+
+@MainActor
+struct RowChangeStatementFactoryUniqueMatchTests {
+    private let columns = ["d", "id", "v"]
+    private let row: [PluginCellValue] = ["2026-10-22", "1", "a"]
+
+    private func factory(_ databaseType: DatabaseType, primaryKeyColumns: [String] = ["d"]) -> RowChangeStatementFactory {
+        RowChangeStatementFactory(
+            tableName: "t",
+            schemaName: nil,
+            columns: columns,
+            primaryKeyColumns: primaryKeyColumns,
+            databaseType: databaseType,
+            pluginDriver: nil
+        )
+    }
+
+    private var edit: RowChange {
+        RowChange(
+            rowID: .existing(0),
+            type: .update,
+            cellChanges: [CellChange(columnIndex: 2, columnName: "v", oldValue: "a", newValue: "b")],
+            originalRow: row
+        )
+    }
+
+    private var delete: RowChange {
+        RowChange(rowID: .existing(0), type: .delete, originalRow: row)
+    }
+
+    private func refusal(_ write: () throws -> [ParameterizedStatement]) -> (kind: RowWriteKind?, reason: String)? {
+        do {
+            let statements = try write()
+            Issue.record("Sent \(statements.map(\.sql))")
+            return nil
+        } catch let DataWriteError.changeRefused(table, kind, reason) {
+            #expect(table == "t")
+            return (kind, reason)
+        } catch {
+            Issue.record("Unexpected error: \(error)")
+            return nil
+        }
+    }
+
+    @Test(arguments: [DatabaseType.clickhouse, .snowflake, .trino])
+    func aDeleteIsRefusedInsteadOfKeyedOnAKeyTheEngineDoesNotKeepUnique(databaseType: DatabaseType) {
+        let refused = refusal {
+            try factory(databaseType).statements(for: [delete], deletedRowIDs: [.existing(0)])
+        }
+        #expect(refused?.kind == .delete)
+        #expect(refused?.reason.contains(databaseType.displayName) == true)
+        #expect(refused?.reason.contains("SQL statement") == true)
+    }
+
+    @Test(arguments: [DatabaseType.clickhouse, .snowflake, .trino])
+    func anEditIsRefusedInsteadOfKeyedOnAKeyTheEngineDoesNotKeepUnique(databaseType: DatabaseType) {
+        let refused = refusal { try factory(databaseType).statements(for: [edit]) }
+        #expect(refused?.kind == .update)
+    }
+
+    @Test
+    func aNewClickHouseRowIsStillInserted() throws {
+        let inserted = RowID.inserted(UUID())
+        let statements = try factory(.clickhouse).statements(
+            for: [RowChange(rowID: inserted, type: .insert)],
+            insertedRowData: [inserted: row],
+            insertedRowIDs: [inserted]
+        )
+        #expect(statements.count == 1)
+        #expect(statements.first?.sql.hasPrefix("INSERT") == true)
+    }
+
+    @Test
+    func aDamengTableWithAPrimaryKeyIsStillWritten() throws {
+        let deleted = RowChange(rowID: .existing(1), type: .delete, originalRow: ["2026-10-22", "2", "a"])
+        let statements = try factory(.dameng, primaryKeyColumns: ["id"]).statements(
+            for: [edit, deleted],
+            deletedRowIDs: [.existing(1)]
+        )
+        #expect(Set(statements.map { String($0.sql.prefix(6)) }) == ["UPDATE", "DELETE"])
+    }
+
+    @Test
+    func aDamengTableWithoutAPrimaryKeyRefusesADelete() {
+        let refused = refusal {
+            try factory(.dameng, primaryKeyColumns: []).statements(for: [delete], deletedRowIDs: [.existing(0)])
+        }
+        #expect(refused?.kind == .delete)
+        #expect(refused?.reason.contains("primary key") == true)
+    }
+
+    @Test
+    func rewindingAClickHouseInsertIsRefused() {
+        let target = DataWriteTarget(database: "default", schema: nil, table: "t")
+        let record = RewindRecord(
+            id: UUID(),
+            historyId: nil,
+            connectionId: UUID(),
+            databaseType: .clickhouse,
+            target: target,
+            capturedAt: Date(timeIntervalSince1970: 0),
+            generatedColumns: [],
+            identityColumns: [],
+            operations: [
+                RowWriteOperation(
+                    kind: .insert,
+                    target: target,
+                    columns: columns,
+                    primaryKeyColumns: ["d"],
+                    preImage: nil,
+                    postImage: row,
+                    writtenColumns: columns,
+                    refusal: nil
+                ),
+            ]
+        )
+        let planner = RewindPlanner(
+            record: record,
+            factory: factory(.clickhouse),
+            queryBuilder: TableQueryBuilder(
+                databaseType: .clickhouse,
+                dialect: PluginManager.shared.sqlDialect(for: .clickhouse),
+                pagination: .offset
+            )
+        )
+
+        #expect(throws: DataWriteError.self) {
+            try planner.plan(currentRows: [RewindCurrentRow(values: row)])
+        }
+    }
+}

changelog.d/3794.changed.md

@@ -0,0 +1 @@
+The grid no longer edits or deletes ClickHouse, Snowflake, Trino or keyless Dameng rows; use a SQL statement instead.

changelog.d/3794.fixed.md

@@ -0,0 +1 @@
+A grid edit or delete on ClickHouse, Snowflake, Trino or a keyless Dameng table could change other rows too.