From 3e7655cf751ed84492911c18fc86dbbbcd7b63ef Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 20 Oct 2026 20:57:23 +0700 Subject: [PATCH 1/3] feat(datagrid): refuse a grid save that would send an UPDATE or DELETE without a WHERE --- .../SQLUnfilteredWrite.swift | 127 ++++++++++++++ .../SQLUnfilteredWriteTests.swift | 133 +++++++++++++++ .../SpannerRowEditSQLRowMatchTests.swift | 77 +++++++++ .../ClickHouseDMLStatementTests.swift | 46 ++++++ .../DynamoDBWriteStatements.swift | 11 +- .../MSSQLStatementGenerator.swift | 15 +- .../SnowflakeStatementGenerator.swift | 3 +- .../TeradataPluginDriver+Editing.swift | 38 +++-- .../TrinoPluginDriver+Editing.swift | 25 +-- .../DataWrite/RowChangeStatementFactory.swift | 33 ++++ .../Core/Utilities/SQL/QueryClassifier.swift | 4 +- ...StatementFactoryUnfilteredWriteTests.swift | 147 +++++++++++++++++ ...QueryClassifierUnfilteredDeleteTests.swift | 29 ++++ .../probes/check-mysql-enum-set-save.sh | 155 ++++++++++++++++++ 14 files changed, 804 insertions(+), 39 deletions(-) create mode 100644 apple/Packages/TableProCore/Sources/TableProSQLGrammar/SQLUnfilteredWrite.swift create mode 100644 apple/Packages/TableProCore/Tests/TableProSQLGrammarTests/SQLUnfilteredWriteTests.swift create mode 100644 apple/Packages/TableProCore/Tests/TableProSpannerCoreTests/SpannerRowEditSQLRowMatchTests.swift create mode 100644 apple/TableProTests/Core/DataWrite/RowChangeStatementFactoryUnfilteredWriteTests.swift create mode 100644 apple/TableProTests/Core/Utilities/SQL/QueryClassifierUnfilteredDeleteTests.swift create mode 100755 apple/scripts/probes/check-mysql-enum-set-save.sh diff --git a/apple/Packages/TableProCore/Sources/TableProSQLGrammar/SQLUnfilteredWrite.swift b/apple/Packages/TableProCore/Sources/TableProSQLGrammar/SQLUnfilteredWrite.swift new file mode 100644 index 0000000000..835d11631c --- /dev/null +++ b/apple/Packages/TableProCore/Sources/TableProSQLGrammar/SQLUnfilteredWrite.swift @@ -0,0 +1,127 @@ +import Foundation + +// A WHERE in a comment, a literal, a quoted identifier or a subquery is not the statement's own. +public enum SQLUnfilteredWrite: Sendable, Equatable { + case update + case delete + + public static func writes(in sql: String, grammar: SQLLexicalGrammar) -> [SQLUnfilteredWrite] { + SQLStatementScanner.executableStatements(in: sql, grammar: grammar).flatMap { statement in + // On T-SQL a statement can begin inside another without a `;`. + SQLUnterminatedStatements.runnable(in: statement.sql, grammar: grammar).flatMap { text in + writes(inStatement: text as NSString, grammar: grammar) + } + } + } + + private static let period = UInt16(UnicodeScalar(".").value) + + private static let servedStatements: Set = ["SELECT", "INSERT", "UPDATE", "DELETE", "MERGE"] + + // `:where` and `$where` are bind parameters on SQLite. + private static let parameterPrefixes: Set = [ + UInt16(UnicodeScalar(":").value), UInt16(UnicodeScalar("$").value), + ] + + private static func writes(inStatement source: NSString, grammar: SQLLexicalGrammar) -> [SQLUnfilteredWrite] { + let tokens = TSQLTokens.read(source, grammar: grammar) + guard let start = tokens.firstIndex(where: { $0.kind != .open }), let first = tokens[start].word else { + return [] + } + let depth = tokens[start].depth + var found: [SQLUnfilteredWrite] = [] + let verb: Int? + switch first { + case "UPDATE", "DELETE": + verb = start + case "WITH": + // A CTE may be named `delete` or `merge`, and on PostgreSQL its body may itself write. A list that does + // not parse is searched whole, which can only find a write that is not there. + let list = commonTableExpressions(in: tokens, with: start) + for body in list?.bodies ?? [] { + found += writes(inStatement: source.substring(with: body) as NSString, grammar: grammar) + } + verb = tokens.indices.dropFirst(list?.main ?? start + 1).first { index in + tokens[index].depth == depth && servedStatements.contains(tokens[index].word ?? "") + } + case "ALTER": + // ClickHouse writes rows with `ALTER TABLE t UPDATE ... WHERE` and `ALTER TABLE t DELETE WHERE`. + guard tokens.indices.contains(start + 1), tokens[start + 1].word == "TABLE" else { return [] } + verb = tokens.indices.dropFirst(start + 2).first { index in + let word = tokens[index].word + return tokens[index].depth == depth && (word == "UPDATE" || word == "DELETE") + && tokens[index - 1].word != "ON" + } + default: + verb = nil + } + if let verb, let write = write(at: verb, in: tokens), !hasOwnWhere(tokens, depth: depth, in: source) { + found.append(write) + } + return found + } + + private static func commonTableExpressions( + in tokens: [TSQLTokens.Token], + with start: Int + ) -> (bodies: [NSRange], main: Int)? { + let depth = tokens[start].depth + var index = start + 1 + var bodies: [NSRange] = [] + func word(_ expected: String) -> Bool { + guard tokens.indices.contains(index), tokens[index].word == expected else { return false } + index += 1 + return true + } + func parentheses() -> NSRange? { + guard tokens.indices.contains(index), tokens[index].kind == .open, + let close = tokens.indices.dropFirst(index + 1).first(where: { + tokens[$0].kind == .close && tokens[$0].depth == depth + }) + else { return nil } + let inside = tokens[index].location + 1 + index = close + 1 + return NSRange(location: inside, length: tokens[close].location - inside) + } + _ = word("RECURSIVE") + while tokens.indices.contains(index) { + index += 1 + _ = parentheses() + guard word("AS") else { return nil } + _ = word("NOT") + _ = word("MATERIALIZED") + guard let body = parentheses() else { return nil } + bodies.append(body) + guard tokens.indices.contains(index), tokens[index].kind == .comma else { break } + index += 1 + } + return tokens.indices.contains(index) ? (bodies, index) : nil + } + + private static func write(at index: Int, in tokens: [TSQLTokens.Token]) -> SQLUnfilteredWrite? { + switch tokens[index].word { + case "UPDATE": + // T-SQL's `UPDATE STATISTICS t` writes no rows. A table named statistics is still updated with a SET. + let next = tokens.indices.contains(index + 1) ? tokens[index + 1].word : nil + let depth = tokens[index].depth + let setsColumns = tokens.dropFirst(index + 1).contains { $0.depth == depth && $0.word == "SET" } + return next == "STATISTICS" && !setsColumns ? nil : .update + case "DELETE": + return .delete + default: + return nil + } + } + + private static func hasOwnWhere(_ tokens: [TSQLTokens.Token], depth: Int, in source: NSString) -> Bool { + tokens.indices.contains { index in + guard tokens[index].depth == depth, tokens[index].word == "WHERE" else { return false } + guard index > 0 else { return true } + let location = tokens[index].location + if location > 0, parameterPrefixes.contains(source.character(at: location - 1)) { return false } + // `t.where` is a column. + let previous = tokens[index - 1] + return previous.kind != .other || source.character(at: previous.location) != period + } + } +} diff --git a/apple/Packages/TableProCore/Tests/TableProSQLGrammarTests/SQLUnfilteredWriteTests.swift b/apple/Packages/TableProCore/Tests/TableProSQLGrammarTests/SQLUnfilteredWriteTests.swift new file mode 100644 index 0000000000..1bf04160cc --- /dev/null +++ b/apple/Packages/TableProCore/Tests/TableProSQLGrammarTests/SQLUnfilteredWriteTests.swift @@ -0,0 +1,133 @@ +import Foundation +import TableProSQLGrammar +import Testing + +struct SQLUnfilteredWriteTests { + private static func grammar(_ typeId: String) -> SQLLexicalGrammar { + SQLLexicalReadings.resolve(databaseTypeId: typeId, declared: nil, session: nil).execution + } + + private static let postgreSQL = grammar("PostgreSQL") + private static let sqlServer = grammar("SQL Server") + private static let clickHouse = grammar("ClickHouse") + private static let mySQL = grammar("MySQL") + + private func first(_ sql: String, _ grammar: SQLLexicalGrammar = postgreSQL) -> SQLUnfilteredWrite? { + SQLUnfilteredWrite.writes(in: sql, grammar: grammar).first + } + + @Test("An UPDATE or DELETE with no WHERE is found", arguments: [ + ("UPDATE t SET a = 1", SQLUnfilteredWrite.update), + ("update \"t\" set \"a\" = $1", .update), + ("DELETE FROM t", .delete), + ("delete from \"s\".\"t\";", .delete), + ("UPDATE t SET a = (SELECT b FROM u WHERE u.id = 1)", .update), + ("DELETE FROM t WHERE_x = 1", .delete), + ("WITH c AS (SELECT id FROM u WHERE id > 1) DELETE FROM t", .delete), + ("UPDATE t SET a = t.where", .update), + ]) + func unfiltered(sql: String, expected: SQLUnfilteredWrite) { + #expect(first(sql) == expected) + } + + @Test("A WHERE the engine never reads as code does not count", arguments: [ + "UPDATE t SET a = 1 -- WHERE id = 1", + "UPDATE t SET a = 1 /* WHERE id = 1 */", + "UPDATE t SET a = ' WHERE id = 1 '", + "UPDATE t SET \" WHERE \" = 1", + "DELETE FROM t -- WHERE id = 1", + "DELETE FROM \"t WHERE id = 1\"", + ]) + func whereOutsideCode(sql: String) { + #expect(first(sql) != nil) + } + + @Test("A statement with a WHERE of its own passes", arguments: [ + "UPDATE t SET a = 1 WHERE id = 1", + "update t set a = 1\nwhere id = 1", + "DELETE FROM t WHERE(id = 1)", + "DELETE FROM \"t\" WHERE \"id\" = $1 OR \"id\" = $2", + "UPDATE t SET a = (SELECT 1) WHERE id = 1 RETURNING *", + "WITH c AS (SELECT 1) UPDATE t SET a = 1 WHERE id = 1", + "INSERT INTO t (a) VALUES (1)", + "SELECT * FROM t", + ]) + func filtered(sql: String) { + #expect(first(sql) == nil) + } + + @Test("Every statement in the text is read") + func everyStatementIsRead() { + #expect(first("UPDATE t SET a = 1 WHERE id = 1; DELETE FROM t") == .delete) + #expect(first("DELETE FROM t WHERE id = 1; UPDATE t SET a = 1 WHERE id = 2") == nil) + #expect(SQLUnfilteredWrite.writes(in: "UPDATE t SET a = 1; DELETE FROM t", grammar: Self.postgreSQL) + == [.update, .delete]) + } + + @Test("A common table expression named like a verb is not the statement") + func commonTableExpressionNames() { + #expect(first("WITH delete AS (SELECT 1) SELECT * FROM delete") == nil) + #expect(first("WITH merge AS (SELECT 1) DELETE FROM items") == .delete) + #expect(first("WITH RECURSIVE update (n) AS (SELECT 1), b AS MATERIALIZED (SELECT 2) UPDATE t SET a = 1") + == .update) + #expect(first("WITH a AS NOT MATERIALIZED (SELECT 1) DELETE FROM t WHERE id IN (SELECT * FROM a)") == nil) + } + + @Test("A common table expression that writes is read as the write it is") + func dataModifyingCommonTableExpression() { + #expect(first("WITH gone AS (DELETE FROM items RETURNING *) SELECT * FROM gone") == .delete) + #expect(first("WITH gone AS (DELETE FROM items WHERE id = 1 RETURNING *) SELECT * FROM gone") == nil) + #expect(first("WITH a AS (SELECT 1), b AS (UPDATE t SET c = 1 RETURNING c) SELECT * FROM b") == .update) + } + + @Test("Only T-SQL's maintenance command is spelled UPDATE STATISTICS without a SET") + func tableNamedStatistics() { + #expect(first("UPDATE statistics SET value = 1") == .update) + #expect(first("UPDATE statistics SET value = 1 WHERE id = 1") == nil) + } + + @Test("A bind parameter named where is not a WHERE") + func parameterNamedWhere() { + let sqlite = Self.grammar("SQLite") + #expect(first("DELETE FROM t RETURNING :where", sqlite) == .delete) + #expect(first("DELETE FROM t RETURNING $where", sqlite) == .delete) + #expect(first("DELETE FROM t WHERE id = :where", sqlite) == nil) + } + + @Test("A statement in parentheses is read at its own depth") + func parenthesizedStatement() { + #expect(first("(DELETE FROM t)") == .delete) + #expect(first("(DELETE FROM t WHERE id = 1)") == nil) + } + + @Test("On SQL Server a statement written without a terminator is read on its own") + func sqlServerUnterminatedStatement() { + #expect(first("UPDATE [t] SET [a] = 1 WHERE [id] = 1 DELETE FROM [t]", Self.sqlServer) == .delete) + #expect(first("DELETE FROM [t] UPDATE [t] SET [a] = 1 WHERE [id] = 1", Self.sqlServer) == .delete) + #expect(first("SET NOCOUNT OFF; UPDATE TOP (1) [t] SET [a] = ? WHERE [a] = ?", Self.sqlServer) == nil) + #expect(first("DELETE TOP (1) FROM [t] WHERE [a] IS NULL", Self.sqlServer) == nil) + #expect(first("UPDATE [t] SET [a] = 1 -- WHERE [id] = 1", Self.sqlServer) == .update) + #expect(first("UPDATE STATISTICS [t]", Self.sqlServer) == nil) + } + + @Test("ClickHouse's ALTER TABLE mutations are read as the write they are") + func clickHouseMutations() { + #expect(first("ALTER TABLE `t` UPDATE `a` = ? WHERE `id` = ?", Self.clickHouse) == nil) + #expect(first("ALTER TABLE `t` DELETE WHERE `id` = ?", Self.clickHouse) == nil) + #expect(first("ALTER TABLE `t` UPDATE `a` = ?", Self.clickHouse) == .update) + #expect(first("ALTER TABLE `t` ON CLUSTER c DELETE", Self.clickHouse) == .delete) + #expect(first("ALTER TABLE `t` UPDATE `a` = '# WHERE'", Self.clickHouse) == .update) + } + + @Test("A table change that names UPDATE or DELETE as a referential action is not a write") + func referentialActionsAreNotWrites() { + #expect(first("ALTER TABLE t ADD FOREIGN KEY (a) REFERENCES u (id) ON DELETE CASCADE", Self.mySQL) == nil) + #expect(first("ALTER TABLE t MODIFY c TIMESTAMP ON UPDATE CURRENT_TIMESTAMP", Self.mySQL) == nil) + } + + @Test("A MySQL comment is closed where MySQL closes it") + func mySQLHashComment() { + #expect(first("UPDATE `t` SET `a` = 1 # WHERE `id` = 1", Self.mySQL) == .update) + #expect(first("UPDATE `t` SET `a` = 1 WHERE `id` = 1", Self.mySQL) == nil) + } +} diff --git a/apple/Packages/TableProCore/Tests/TableProSpannerCoreTests/SpannerRowEditSQLRowMatchTests.swift b/apple/Packages/TableProCore/Tests/TableProSpannerCoreTests/SpannerRowEditSQLRowMatchTests.swift new file mode 100644 index 0000000000..9c3eb49e2d --- /dev/null +++ b/apple/Packages/TableProCore/Tests/TableProSpannerCoreTests/SpannerRowEditSQLRowMatchTests.swift @@ -0,0 +1,77 @@ +import TableProSpannerCore +import Testing + +struct SpannerRowEditSQLRowMatchTests { + private let columns = ["tenant", "id", "name"] + private let originalRow: [SpannerCell] = [.text("acme"), .text("7"), .text("a")] + + private func statements( + _ changes: [SpannerRowChange], + columns: [String]? = nil, + primaryKey: [String] = ["tenant", "id"], + deletedRows: Set = [] + ) -> [SpannerRenderedStatement]? { + SpannerRowEditSQL.statements( + schema: "", + table: "t", + columns: columns ?? self.columns, + primaryKey: primaryKey, + changes: changes, + insertedRows: [:], + deletedRows: deletedRows, + insertedRowIndices: [], + dialect: .postgreSQL + ) + } + + private func rename(row: [SpannerCell]? = nil) -> SpannerRowChange { + SpannerRowChange( + rowIndex: 0, + kind: .update, + cellChanges: [SpannerRowChange.CellChange(column: "name", value: .text("b"))], + originalRow: row ?? originalRow + ) + } + + private func deletion(row: [SpannerCell]? = nil) -> SpannerRowChange { + SpannerRowChange(rowIndex: 1, kind: .delete, originalRow: row ?? originalRow) + } + + @Test("An update and a delete name the row by every key column") + func writesNameTheRowByItsKey() { + #expect(statements([rename(), deletion()], deletedRows: [1]) == [ + SpannerRenderedStatement( + sql: #"UPDATE "t" SET "name" = ? WHERE "tenant" = ? AND "id" = ?"#, + parameters: [.text("b"), .text("acme"), .text("7")] + ), + SpannerRenderedStatement( + sql: #"DELETE FROM "t" WHERE "tenant" = ? AND "id" = ?"#, + parameters: [.text("acme"), .text("7")] + ), + ]) + } + + @Test("A table with no key writes nothing rather than a statement that could reach other rows") + func keylessTableIsRefused() { + #expect(statements([rename()], primaryKey: []) == nil) + #expect(statements([deletion()], primaryKey: [], deletedRows: [1]) == nil) + } + + @Test("A result that leaves out a key column writes nothing") + func missingKeyColumnIsRefused() { + let partial = ["id", "name"] + let row: [SpannerCell] = [.text("7"), .text("a")] + #expect(statements([rename(row: row)], columns: partial) == nil) + #expect(statements([deletion(row: row)], columns: partial, deletedRows: [1]) == nil) + } + + @Test("A change with no row read back writes nothing") + func changeWithoutOriginalRowIsRefused() { + let change = SpannerRowChange( + rowIndex: 0, + kind: .update, + cellChanges: [SpannerRowChange.CellChange(column: "name", value: .text("b"))] + ) + #expect(statements([change]) == nil) + } +} diff --git a/apple/Plugins/ClickHouseDriverTests/ClickHouseDMLStatementTests.swift b/apple/Plugins/ClickHouseDriverTests/ClickHouseDMLStatementTests.swift index ac13316d74..0fe826bef9 100644 --- a/apple/Plugins/ClickHouseDriverTests/ClickHouseDMLStatementTests.swift +++ b/apple/Plugins/ClickHouseDriverTests/ClickHouseDMLStatementTests.swift @@ -104,6 +104,52 @@ struct ClickHouseDMLStatementTests { #expect(!sql.contains("ALTER TABLE")) } + @Test("An update and a delete match the row on every column it was read with", arguments: [["id"], []]) + func writesMatchTheWholeRow(primaryKeyColumns: [String]) { + let original: [PluginCellValue] = [.text("1"), .null] + let statements = driver.generateStatements( + table: table, + columns: columns, + primaryKeyColumns: primaryKeyColumns, + changes: [ + PluginRowChange( + rowIndex: 0, + type: .update, + cellChanges: [(columnIndex: 1, columnName: "name", oldValue: .null, newValue: .text("Bob"))], + originalRow: original + ), + PluginRowChange(rowIndex: 1, type: .delete, cellChanges: [], originalRow: original), + ], + insertedRowData: [:], + deletedRowIndices: [1], + insertedRowIndices: [] + ) + + #expect(statements?.map { $0.statement } == [ + "ALTER TABLE `users` UPDATE `name` = ? WHERE `id` = ? AND `name` IS NULL", + "ALTER TABLE `users` DELETE WHERE `id` = ? AND `name` IS NULL", + ]) + #expect(statements?.map { $0.parameters } == [[.text("Bob"), .text("1")], [.text("1")]]) + } + + @Test("A change with no row to match writes nothing rather than an unfiltered mutation") + func changeWithoutOriginalRowWritesNothing() { + let statements = driver.generateStatements( + table: table, + columns: columns, + primaryKeyColumns: ["id"], + changes: [ + change(.update, cells: [(columnIndex: 1, columnName: "name", oldValue: .text("a"), newValue: .text("b"))]), + PluginRowChange(rowIndex: 1, type: .delete, cellChanges: [], originalRow: nil), + ], + insertedRowData: [:], + deletedRowIndices: [1], + insertedRowIndices: [] + ) + + #expect(statements == nil) + } + /// A row the caller did not mark deleted is skipped, so a stale change cannot delete anything. @Test("A delete not listed in deletedRowIndices produces nothing") func unlistedDeleteIsSkipped() { diff --git a/apple/Plugins/DynamoDBDriverPlugin/DynamoDBWriteStatements.swift b/apple/Plugins/DynamoDBDriverPlugin/DynamoDBWriteStatements.swift index 913fdb9e3d..e925e2f8c6 100644 --- a/apple/Plugins/DynamoDBDriverPlugin/DynamoDBWriteStatements.swift +++ b/apple/Plugins/DynamoDBDriverPlugin/DynamoDBWriteStatements.swift @@ -109,9 +109,8 @@ struct DynamoDBWriteStatements { guards.append("\(name) = ?") guardValues.append(cell.oldValue) } - guard !assignments.isEmpty || !removals.isEmpty else { return nil } + guard !assignments.isEmpty || !removals.isEmpty, let key = keyCondition(original) else { return nil } - let key = keyCondition(original) var statement = "UPDATE \(DynamoDBStatement.quote(table))" if !assignments.isEmpty { statement += " SET " + assignments.joined(separator: ", ") @@ -124,18 +123,18 @@ struct DynamoDBWriteStatements { } private func delete(_ change: PluginRowChange) -> (statement: String, parameters: [PluginCellValue])? { - guard let original = change.originalRow else { return nil } - let key = keyCondition(original) + guard let original = change.originalRow, let key = keyCondition(original) else { return nil } let statement = "DELETE FROM \(DynamoDBStatement.quote(table)) WHERE " + key.terms.joined(separator: " AND ") + " RETURNING ALL OLD *" return (statement, key.values) } - private func keyCondition(_ row: [PluginCellValue]) -> (terms: [String], values: [PluginCellValue]) { + private func keyCondition(_ row: [PluginCellValue]) -> (terms: [String], values: [PluginCellValue])? { + guard !keyColumns.isEmpty else { return nil } var terms: [String] = [] var values: [PluginCellValue] = [] for column in keyColumns { - guard let index = columns.firstIndex(of: column), index < row.count else { continue } + guard let index = columns.firstIndex(of: column), index < row.count else { return nil } terms.append("\(DynamoDBStatement.quote(column)) = ?") values.append(row[index]) } diff --git a/apple/Plugins/MSSQLDriverPlugin/MSSQLStatementGenerator.swift b/apple/Plugins/MSSQLDriverPlugin/MSSQLStatementGenerator.swift index f7d472f612..8c115e1caf 100644 --- a/apple/Plugins/MSSQLDriverPlugin/MSSQLStatementGenerator.swift +++ b/apple/Plugins/MSSQLDriverPlugin/MSSQLStatementGenerator.swift @@ -152,7 +152,12 @@ struct MSSQLStatementGenerator { var conditions: [String] = [] var parameters: [PluginCellValue] = [] for column in matchColumns { - guard let index = columns.firstIndex(of: column), index < originalRow.count else { continue } + guard let index = columns.firstIndex(of: column), index < originalRow.count else { + throw PluginRowWriteRefusal( + rowIndex: rowIndex, + reason: isKeyless ? Self.rowNotLoadedReason : Self.keyNotLoadedReason + ) + } if isKeyless, context.rowMatchExcludedColumns.contains(column) { throw PluginRowWriteRefusal(rowIndex: rowIndex, reason: Self.unmatchableReason(column)) } @@ -181,6 +186,14 @@ struct MSSQLStatementGenerator { String(format: String(localized: "The server fills in %@, so it cannot be given a value."), column) } + private static let keyNotLoadedReason = String( + localized: "The row's primary key is not loaded, so the row cannot be found." + ) + + private static let rowNotLoadedReason = String( + localized: "The table has no primary key, so this row cannot be identified." + ) + private static func unmatchableReason(_ column: String) -> String { String( format: String(localized: "This table has no primary key, and %@ cannot be compared to find the row."), diff --git a/apple/Plugins/SnowflakeDriverPlugin/SnowflakeStatementGenerator.swift b/apple/Plugins/SnowflakeDriverPlugin/SnowflakeStatementGenerator.swift index 2ffb98e00d..121b0f03c7 100644 --- a/apple/Plugins/SnowflakeDriverPlugin/SnowflakeStatementGenerator.swift +++ b/apple/Plugins/SnowflakeDriverPlugin/SnowflakeStatementGenerator.swift @@ -122,7 +122,8 @@ struct SnowflakeStatementGenerator { var parameters: [PluginCellValue] = [] for column in keyColumns { - guard let index = columns.firstIndex(of: column), index < originalRow.count else { continue } + // Leaving a column out of the match reaches every row that shares the rest. + guard let index = columns.firstIndex(of: column), index < originalRow.count else { return nil } if Self.isSemiStructured(typeName(for: column)) { continue } let value = originalRow[index] if case .null = value { diff --git a/apple/Plugins/TeradataDriverPlugin/TeradataPluginDriver+Editing.swift b/apple/Plugins/TeradataDriverPlugin/TeradataPluginDriver+Editing.swift index 3ca74332b4..9700105918 100644 --- a/apple/Plugins/TeradataDriverPlugin/TeradataPluginDriver+Editing.swift +++ b/apple/Plugins/TeradataDriverPlugin/TeradataPluginDriver+Editing.swift @@ -48,13 +48,15 @@ extension TeradataPluginDriver { isExpression: expressionColumns.contains($0.columnIndex) ) } - guard !assignments.isEmpty else { continue } - let whereClause = keyPredicate(primaryKeyColumns, columns: columns, change: change) - statements.append(("UPDATE \(target) SET \(assignments.joined(separator: ", "))\(whereClause)", [])) + guard !assignments.isEmpty, + let whereClause = keyPredicate(primaryKeyColumns, columns: columns, change: change) + else { continue } + statements.append(("UPDATE \(target) SET \(assignments.joined(separator: ", ")) WHERE \(whereClause)", [])) case .delete: - guard deletedRowIndices.contains(change.rowIndex) else { continue } - let whereClause = keyPredicate(primaryKeyColumns, columns: columns, change: change) - statements.append(("DELETE FROM \(target)\(whereClause)", [])) + guard deletedRowIndices.contains(change.rowIndex), + let whereClause = keyPredicate(primaryKeyColumns, columns: columns, change: change) + else { continue } + statements.append(("DELETE FROM \(target) WHERE \(whereClause)", [])) } } return statements.isEmpty ? nil : statements @@ -69,24 +71,26 @@ extension TeradataPluginDriver { return currentDatabaseName } - private func keyPredicate(_ primaryKeyColumns: [String], columns: [String], change: PluginRowChange) -> String { + private func keyPredicate(_ primaryKeyColumns: [String], columns: [String], change: PluginRowChange) -> String? { let keyColumns = primaryKeyColumns.isEmpty ? columns : primaryKeyColumns - let conditions = keyColumns.map { column -> String in - let value = oldValue(for: column, columns: columns, change: change) - if case .null = value { return "\(quote(column)) IS NULL" } - return "\(quote(column)) = \(sqlLiteral(value))" + var conditions: [String] = [] + for column in keyColumns { + // A value that was never loaded is not NULL, and matching it as one finds the wrong rows. + guard let value = oldValue(for: column, columns: columns, change: change) else { return nil } + if case .null = value { + conditions.append("\(quote(column)) IS NULL") + } else { + conditions.append("\(quote(column)) = \(sqlLiteral(value))") + } } - return conditions.isEmpty ? "" : " WHERE " + conditions.joined(separator: " AND ") + return conditions.isEmpty ? nil : conditions.joined(separator: " AND ") } - private func oldValue(for column: String, columns: [String], change: PluginRowChange) -> PluginCellValue { + private func oldValue(for column: String, columns: [String], change: PluginRowChange) -> PluginCellValue? { if let originalRow = change.originalRow, let index = columns.firstIndex(of: column), index < originalRow.count { return originalRow[index] } - if let cell = change.cellChanges.first(where: { $0.columnName == column }) { - return cell.oldValue - } - return .null + return change.cellChanges.first(where: { $0.columnName == column })?.oldValue } } diff --git a/apple/Plugins/TrinoDriverPlugin/TrinoPluginDriver+Editing.swift b/apple/Plugins/TrinoDriverPlugin/TrinoPluginDriver+Editing.swift index fa882a39d2..70bceb46a8 100644 --- a/apple/Plugins/TrinoDriverPlugin/TrinoPluginDriver+Editing.swift +++ b/apple/Plugins/TrinoDriverPlugin/TrinoPluginDriver+Editing.swift @@ -48,16 +48,16 @@ extension TrinoPluginDriver { let assignments = change.cellChanges.map { TrinoColumnValue(name: $0.columnName, value: TrinoRowEditValues.trinoValue($0.newValue), typeName: typeName($0.columnName)) } - let keys = keyColumns(primaryKeyColumns, columns: columns, change: change, typeName: typeName) - if let sql = TrinoRowEditSQL.update(qualifiedTable: target, assignments: assignments, keyColumns: keys) { - statements.append((sql, [])) - } + guard let keys = keyColumns(primaryKeyColumns, columns: columns, change: change, typeName: typeName), + let sql = TrinoRowEditSQL.update(qualifiedTable: target, assignments: assignments, keyColumns: keys) + else { continue } + statements.append((sql, [])) case .delete: guard deletedRowIndices.contains(change.rowIndex) else { continue } - let keys = keyColumns(primaryKeyColumns, columns: columns, change: change, typeName: typeName) - if let sql = TrinoRowEditSQL.delete(qualifiedTable: target, keyColumns: keys) { - statements.append((sql, [])) - } + guard let keys = keyColumns(primaryKeyColumns, columns: columns, change: change, typeName: typeName), + let sql = TrinoRowEditSQL.delete(qualifiedTable: target, keyColumns: keys) + else { continue } + statements.append((sql, [])) } } return statements.isEmpty ? nil : statements @@ -68,12 +68,15 @@ extension TrinoPluginDriver { columns: [String], change: PluginRowChange, typeName: (String) -> String - ) -> [TrinoColumnValue] { + ) -> [TrinoColumnValue]? { let keyNames = primaryKeyColumns.isEmpty ? columns : primaryKeyColumns - return keyNames.compactMap { column in + var keys: [TrinoColumnValue] = [] + for column in keyNames { + // Leaving a column out of the match reaches every row that shares the rest. guard let value = originalValue(column, columns: columns, change: change) else { return nil } - return TrinoColumnValue(name: column, value: TrinoRowEditValues.trinoValue(value), typeName: typeName(column)) + keys.append(TrinoColumnValue(name: column, value: TrinoRowEditValues.trinoValue(value), typeName: typeName(column))) } + return keys } private func originalValue(_ column: String, columns: [String], change: PluginRowChange) -> PluginCellValue? { diff --git a/apple/TablePro/Core/DataWrite/RowChangeStatementFactory.swift b/apple/TablePro/Core/DataWrite/RowChangeStatementFactory.swift index 989c745ca4..ca7ed17e4b 100644 --- a/apple/TablePro/Core/DataWrite/RowChangeStatementFactory.swift +++ b/apple/TablePro/Core/DataWrite/RowChangeStatementFactory.swift @@ -13,6 +13,7 @@ import Foundation import TableProPluginKit +import TableProSQLGrammar /// The statements that put deleted rows back, with what has to run around them outside the transaction. struct RestoreStatements { @@ -316,6 +317,7 @@ struct RowChangeStatementFactory { Set(restorable.indices.filter { absent.contains(restorable[$0]) }) } ) { + try refuseUnfilteredWrites(restored.map { $0.statement }) return RestoreStatements(statements: restored.map { ParameterizedStatement(sql: $0.statement, parameters: $0.parameters.map(\.asAny)) }) @@ -435,6 +437,7 @@ struct RowChangeStatementFactory { throw DataWriteError.changeRefused(table: tableName, kind: nil, reason: error.localizedDescription) } + try refuseUnfilteredWrites(writes.map(\.statement)) let unwritten = RowWriteCoverage.unwrittenChanges( changes, deletedRowIDs: deletedRowIDs, @@ -452,6 +455,18 @@ struct RowChangeStatementFactory { } } + // The host's own generator never writes one, so only a driver's statements are read. + private func refuseUnfilteredWrites(_ statements: [String]) throws { + guard PluginManager.shared.editorLanguage(for: databaseType) == .sql else { return } + let readings = databaseType.lexicalReadings + for sql in statements { + for grammar in readings.distinct(for: sql) { + guard let write = SQLUnfilteredWrite.writes(in: sql, grammar: grammar).first else { continue } + throw DataWriteError.changeRefused(table: tableName, kind: write.kind, reason: write.reason) + } + } + } + /// A keyless match can find nothing and still succeed, so it is held to its row where the engine's count is real. /// Only a statement naming one row: the default `generateRowWrites` puts every row on its first statement. private func keylessRowCount(of write: PluginRowWrite, keyed: PluginKeyedChanges) -> Int? { @@ -540,6 +555,24 @@ struct PluginKeyedChanges { } } +private extension SQLUnfilteredWrite { + var kind: RowWriteKind { + switch self { + case .update: return .update + case .delete: return .delete + } + } + + var reason: String { + switch self { + case .update: + return String(localized: "The driver wrote an UPDATE with no WHERE clause, which would change every row.") + case .delete: + return String(localized: "The driver wrote a DELETE with no WHERE clause, which would remove every row.") + } + } +} + private extension PluginRowChange { init(_ change: RowChange, key: Int) { self.init( diff --git a/apple/TablePro/Core/Utilities/SQL/QueryClassifier.swift b/apple/TablePro/Core/Utilities/SQL/QueryClassifier.swift index f44e6ab55d..464bd4da33 100644 --- a/apple/TablePro/Core/Utilities/SQL/QueryClassifier.swift +++ b/apple/TablePro/Core/Utilities/SQL/QueryClassifier.swift @@ -173,9 +173,7 @@ enum QueryClassifier { if runsProceduralBlock(statement, grammar: grammar) { return proceduralBlockDeletesEverything(statement, grammar: grammar) } - let code = SQLCodeProjection.code(of: statement, grammar: grammar).uppercased() - guard leadingCodeKeyword(code) == "DELETE" else { return false } - return !hasWhereClause(code: code) + return SQLUnfilteredWrite.writes(in: statement, grammar: grammar).contains(.delete) } static func isExplainStatement(_ sql: String) -> Bool { diff --git a/apple/TableProTests/Core/DataWrite/RowChangeStatementFactoryUnfilteredWriteTests.swift b/apple/TableProTests/Core/DataWrite/RowChangeStatementFactoryUnfilteredWriteTests.swift new file mode 100644 index 0000000000..4eb4cb6e17 --- /dev/null +++ b/apple/TableProTests/Core/DataWrite/RowChangeStatementFactoryUnfilteredWriteTests.swift @@ -0,0 +1,147 @@ +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +private final class RestoreStubDriver: PluginDriverStub, @unchecked Sendable { + let restore: String + + init(restore: String) { + self.restore = restore + } + + func generateIdentityPreservingInsert( + table: String, + schema: String?, + columns: [String], + primaryKeyColumns: [String], + rows: [[PluginCellValue]], + absentCells: [Int: Set] + ) -> [(statement: String, parameters: [PluginCellValue])]? { + [(statement: restore, parameters: [])] + } + + func quoteIdentifier(_ name: String) -> String { "[\(name)]" } +} + +@MainActor +struct RowChangeStatementFactoryUnfilteredWriteTests { + private let columns = ["id", "name"] + + private func factory( + databaseType: DatabaseType = .mssql, + primaryKeyColumns: [String] = ["id"], + driver: any PluginDatabaseDriver + ) -> RowChangeStatementFactory { + RowChangeStatementFactory( + tableName: "items", + schemaName: nil, + columns: columns, + primaryKeyColumns: primaryKeyColumns, + databaseType: databaseType, + pluginDriver: driver + ) + } + + private func nameEdit() -> RowChange { + RowChange( + rowID: .existing(0), + type: .update, + cellChanges: [CellChange(columnIndex: 1, columnName: "name", oldValue: "a", newValue: "z")], + originalRow: [.text("1"), "a"] + ) + } + + private func deletion() -> RowChange { + RowChange(rowID: .existing(0), type: .delete, originalRow: [.text("1"), "a"]) + } + + private func writing(_ statement: String) -> RowWriteStubDriver { + RowWriteStubDriver { changes, _, _, _ in + [PluginRowWrite(statement: statement, rowIndices: changes.map(\.rowIndex))] + } + } + + private static let updateReason = "The driver wrote an UPDATE with no WHERE clause, which would change every row." + private static let deleteReason = "The driver wrote a DELETE with no WHERE clause, which would remove every row." + + @Test("A driver's UPDATE with no WHERE refuses the save and names the table") + func updateWithoutWhereRefuses() { + do { + _ = try factory(driver: writing("UPDATE [items] SET [name] = ?")).statements(for: [nameEdit()]) + Issue.record("the save was not refused") + } catch let error as DataWriteError { + #expect(error == .changeRefused(table: "items", kind: .update, reason: Self.updateReason)) + #expect(error.errorDescription?.hasPrefix("Cannot save the edited row in 'items'.") == true) + #expect(error.recoverySuggestion?.hasPrefix("Nothing was saved") == true) + } catch { + Issue.record("unexpected error \(error)") + } + } + + @Test("A driver's DELETE with no WHERE refuses the save") + func deleteWithoutWhereRefuses() { + let delete = deletion() + #expect(throws: DataWriteError.changeRefused(table: "items", kind: .delete, reason: Self.deleteReason)) { + _ = try factory(driver: writing("DELETE FROM [items]")).statements( + for: [delete], deletedRowIDs: [delete.rowID] + ) + } + } + + @Test("A WHERE the server never reads does not count", arguments: [ + "UPDATE [items] SET [name] = ? -- WHERE [id] = 1", + "UPDATE [items] SET [name] = ? /* WHERE [id] = 1 */", + "UPDATE [items] SET [name] = ' WHERE [id] = 1'", + "UPDATE [items] SET [name] = (SELECT TOP (1) [name] FROM [other] WHERE [id] = 1)", + "UPDATE [items] SET [name] = ? WHERE [id] = 1; UPDATE [items] SET [name] = ?", + ]) + func whereOutsideTheStatementRefuses(statement: String) { + #expect(throws: DataWriteError.changeRefused(table: "items", kind: .update, reason: Self.updateReason)) { + _ = try factory(driver: writing(statement)).statements(for: [nameEdit()]) + } + } + + @Test("A statement with a WHERE of its own is sent as written") + func filteredStatementPasses() throws { + let statement = "UPDATE [items] SET [name] = ? WHERE [id] = ?" + let statements = try factory(driver: writing(statement)).statements(for: [nameEdit()]) + #expect(statements.map(\.sql) == [statement]) + } + + @Test("A driver built before generateRowWrites is held to it too") + func legacyDriverIsHeldToIt() { + let driver = LegacyStatementStubDriver { changes, _, _, _ in + changes.isEmpty ? nil : [(statement: "UPDATE \"items\" SET \"name\" = 'z'", parameters: [])] + } + #expect(throws: DataWriteError.changeRefused(table: "items", kind: .update, reason: Self.updateReason)) { + _ = try factory(databaseType: .oracle, driver: driver).statements(for: [nameEdit()]) + } + } + + @Test("ClickHouse's ALTER TABLE mutation with no WHERE refuses the save") + func clickHouseMutationWithoutWhereRefuses() { + #expect(throws: DataWriteError.changeRefused(table: "items", kind: .update, reason: Self.updateReason)) { + _ = try factory(databaseType: .clickhouse, driver: writing("ALTER TABLE `items` UPDATE `name` = ?")) + .statements(for: [nameEdit()]) + } + } + + @Test("A driver whose statements are not SQL is not read as SQL") + func nonSQLEngineIsNotParsed() throws { + let statements = try factory( + databaseType: DatabaseType(rawValue: "MongoDB"), + primaryKeyColumns: ["_id"], + driver: writing("DELETE FROM items") + ).statements(for: [nameEdit()]) + #expect(statements.map(\.sql) == ["DELETE FROM items"]) + } + + @Test("A restore a driver writes is held to it too") + func restoreIsHeldToIt() { + #expect(throws: DataWriteError.changeRefused(table: "items", kind: .delete, reason: Self.deleteReason)) { + _ = try factory(driver: RestoreStubDriver(restore: "DELETE FROM [items]")) + .restoreStatements(rows: [[.text("1"), "a"]]) + } + } +} diff --git a/apple/TableProTests/Core/Utilities/SQL/QueryClassifierUnfilteredDeleteTests.swift b/apple/TableProTests/Core/Utilities/SQL/QueryClassifierUnfilteredDeleteTests.swift new file mode 100644 index 0000000000..f36a93b4a9 --- /dev/null +++ b/apple/TableProTests/Core/Utilities/SQL/QueryClassifierUnfilteredDeleteTests.swift @@ -0,0 +1,29 @@ +import Foundation +@testable import TablePro +import Testing + +struct QueryClassifierUnfilteredDeleteTests { + @Test("A DELETE whose only WHERE is not its own still asks first", arguments: [ + "WITH old AS (SELECT id FROM archive WHERE id > 0) DELETE FROM orders", + "DELETE FROM orders USING (SELECT id FROM archive WHERE id > 0) a", + "DELETE FROM orders RETURNING (SELECT 1 WHERE true)", + ]) + func deleteWithoutItsOwnWhereIsDangerous(sql: String) { + #expect(QueryClassifier.isDangerousQuery(sql, databaseType: .postgresql)) + } + + @Test("A DELETE with a WHERE of its own does not", arguments: [ + "DELETE FROM orders WHERE(id = 1)", + "WITH old AS (SELECT id FROM archive) DELETE FROM orders WHERE id IN (SELECT id FROM old)", + "DELETE FROM orders o WHERE o.id IN (SELECT id FROM archive WHERE id > 0)", + ]) + func deleteWithItsOwnWhereIsNotDangerous(sql: String) { + #expect(!QueryClassifier.isDangerousQuery(sql, databaseType: .postgresql)) + } + + @Test("On SQL Server a WHERE of the statement after the DELETE is not the DELETE's") + func sqlServerFollowingStatementWhere() { + #expect(QueryClassifier.isDangerousQuery("DELETE FROM [orders] SELECT 1 WHERE 1 = 1", databaseType: .mssql)) + #expect(!QueryClassifier.isDangerousQuery("DELETE FROM [orders] WHERE [id] = 1 SELECT 1", databaseType: .mssql)) + } +} diff --git a/apple/scripts/probes/check-mysql-enum-set-save.sh b/apple/scripts/probes/check-mysql-enum-set-save.sh new file mode 100755 index 0000000000..48f9b40368 --- /dev/null +++ b/apple/scripts/probes/check-mysql-enum-set-save.sh @@ -0,0 +1,155 @@ +#!/usr/bin/env bash +# +# Check that a grid save writes ENUM and SET values to a live MySQL or MariaDB server and that they read back. +# +# The save sends what SQLStatementGenerator writes for MySQL, `UPDATE `t` SET `c` = ? WHERE `id` = ?` on a keyed +# table and a match on every column on a keyless one, as a prepared statement whose values are bound as text +# (MariaDBPluginConnection binds MYSQL_TYPE_STRING). This runs the same statements through PREPARE and EXECUTE +# USING, then reads each cell back: one ENUM value, several SET members, an empty SET, NULL, and a value with a +# quote. The SET editor commits its members comma-joined in declared order, and an empty selection as ''. +# +# Usage: +# scripts/probes/check-mysql-enum-set-save.sh [host] [port] [user] +# +# Needs the mysql client and an account that may create and drop a database. The password, if any, comes from +# MYSQL_PWD. Exits non-zero on a disagreement and 3 when no server answers. + +set -uo pipefail + +HOST="${1:-127.0.0.1}" +PORT="${2:-3306}" +USER_NAME="${3:-root}" +DATABASE="tablepro_enum_set_check_$$" + +command -v mysql > /dev/null || { + echo "mysql client not found" >&2 + exit 3 +} + +MYSQL=(mysql --no-defaults -h "$HOST" -P "$PORT" -u "$USER_NAME" -N -B --default-character-set=utf8mb4) +VERSION="$("${MYSQL[@]}" -e "SELECT VERSION()" 2> /dev/null)" || { + echo "no MySQL at $HOST:$PORT for $USER_NAME" >&2 + exit 3 +} +echo "server $VERSION" + +"${MYSQL[@]}" -e "CREATE DATABASE \`$DATABASE\`" || exit 3 +trap '"${MYSQL[@]}" -e "DROP DATABASE IF EXISTS \`$DATABASE\`"' EXIT + +"${MYSQL[@]}" "$DATABASE" -e " + CREATE TABLE keyed ( + id INT PRIMARY KEY, + e ENUM('small', 'large', 'it''s') NULL, + s SET('red', 'green', 'blue', 'it''s') NULL + ); + CREATE TABLE keyless ( + e ENUM('small', 'large', 'it''s') NULL, + s SET('red', 'green', 'blue', 'it''s') NULL + ); + INSERT INTO keyed VALUES (1, 'small', 'red');" || exit 1 + +failures=0 + +report() { + local label="$1" expected="$2" actual="$3" + if [ "$expected" = "$actual" ]; then + echo "ok $label" + else + echo "FAIL $label: expected $expected, got $actual" + failures=$((failures + 1)) + fi +} + +hex() { + printf '%s' "$1" | od -An -tx1 | tr -d ' \n' | tr 'a-f' 'A-F' +} + +# A bound value as a user variable: NULL stays SQL NULL, text becomes a literal with its quotes doubled. +bind() { + local quote="'" + if [ "$1" = NULL ]; then + printf 'NULL' + else + printf "'%s'" "${1//$quote/$quote$quote}" + fi +} + +expected_cell() { + if [ "$1" = NULL ]; then printf 'SQL-NULL'; else hex "$1"; fi +} + +# The statements run in one session, as the save runs them, and ROW_COUNT() reports what EXECUTE matched. +run() { + "${MYSQL[@]}" "$DATABASE" -e "$1" +} + +read_keyed() { + run "SELECT CONCAT(IFNULL(HEX(e), 'SQL-NULL'), ' ', IFNULL(HEX(s), 'SQL-NULL')) FROM keyed WHERE id = 1" +} + +keyed_update() { + local label="$1" enum_value="$2" set_value="$3" stored_set="$4" + run "SET @e = $(bind "$enum_value"), @s = $(bind "$set_value"), @id = 1; + PREPARE w FROM 'UPDATE \`keyed\` SET \`e\` = ?, \`s\` = ? WHERE \`id\` = ?'; + EXECUTE w USING @e, @s, @id;" > /dev/null 2>&1 || { + report "$label" "saved" "refused" + return + } + report "$label" "$(expected_cell "$enum_value") $(expected_cell "$stored_set")" "$(read_keyed)" +} + +keyed_update "one ENUM value and one SET member" large green green +keyed_update "several SET members in declared order" small 'red,blue' 'red,blue' +keyed_update "SET members typed out of order are stored in declared order" small 'blue,red' 'red,blue' +keyed_update "an empty SET is an empty string, not NULL" large '' '' +keyed_update "NULL in both" NULL NULL NULL +keyed_update "a value with a quote" "it's" "red,it's" "red,it's" + +undeclared="$(run "SET @e = 'purple', @id = 1; + PREPARE w FROM 'UPDATE \`keyed\` SET \`e\` = ? WHERE \`id\` = ?'; + EXECUTE w USING @e, @id;" 2>&1)" +case "$undeclared" in + *1265*) report "an undeclared ENUM value is refused, not stored as ''" refused refused ;; + *) report "an undeclared ENUM value is refused, not stored as ''" refused "${undeclared:-saved}" ;; +esac + +# A keyless row is matched on every column, `IS NULL` for a NULL, and held to one row. +keyless_case() { + local label="$1" enum_value="$2" set_value="$3" + run "DELETE FROM keyless; INSERT INTO keyless VALUES ($(bind "$enum_value"), $(bind "$set_value"))" > /dev/null + local e_match="\`e\` = ?" s_match="\`s\` = ?" using=() + if [ "$enum_value" = NULL ]; then e_match="\`e\` IS NULL"; else using+=("@oe"); fi + if [ "$set_value" = NULL ]; then s_match="\`s\` IS NULL"; else using+=("@os"); fi + local update_using="@n" delete_using="" + for variable in ${using[@]+"${using[@]}"}; do + update_using="$update_using, $variable" + delete_using="${delete_using:+$delete_using, }$variable" + done + local prefix="SET @n = 'large', @oe = $(bind "$enum_value"), @os = $(bind "$set_value");" + local updated deleted + updated="$(run "$prefix + PREPARE w FROM 'UPDATE \`keyless\` SET \`e\` = ? WHERE $e_match AND $s_match'; + EXECUTE w USING $update_using; SELECT ROW_COUNT();")" + report "$label: the update finds the row" 1 "$updated" + run "UPDATE keyless SET e = $(bind "$enum_value")" > /dev/null + if [ -n "$delete_using" ]; then + deleted="$(run "$prefix + PREPARE d FROM 'DELETE FROM \`keyless\` WHERE ($e_match AND $s_match)'; + EXECUTE d USING $delete_using; SELECT ROW_COUNT();")" + else + deleted="$(run "DELETE FROM \`keyless\` WHERE ($e_match AND $s_match); SELECT ROW_COUNT();")" + fi + report "$label: the delete finds the row" 1 "$deleted" +} + +keyless_case "keyless, one value each" small green +keyless_case "keyless, several SET members" small 'red,blue' +keyless_case "keyless, an empty SET" small '' +keyless_case "keyless, NULL" NULL NULL +keyless_case "keyless, a value with a quote" "it's" "it's" + +if [ "$failures" -gt 0 ]; then + echo "$failures check(s) failed" + exit 1 +fi +echo "all checks passed" From e977e5f473e1823ce7b20faebadf8dc89604be4e Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 20 Oct 2026 21:00:27 +0700 Subject: [PATCH 2/3] docs(changelog): add the fragments for #3700 --- apple/changelog.d/3700.added.md | 1 + apple/changelog.d/3700.fixed.1.md | 1 + apple/changelog.d/3700.fixed.md | 1 + 3 files changed, 3 insertions(+) create mode 100644 apple/changelog.d/3700.added.md create mode 100644 apple/changelog.d/3700.fixed.1.md create mode 100644 apple/changelog.d/3700.fixed.md diff --git a/apple/changelog.d/3700.added.md b/apple/changelog.d/3700.added.md new file mode 100644 index 0000000000..691609042f --- /dev/null +++ b/apple/changelog.d/3700.added.md @@ -0,0 +1 @@ +A grid save stops before sending an UPDATE or DELETE with no WHERE clause, and names the table. diff --git a/apple/changelog.d/3700.fixed.1.md b/apple/changelog.d/3700.fixed.1.md new file mode 100644 index 0000000000..3c69c887fd --- /dev/null +++ b/apple/changelog.d/3700.fixed.1.md @@ -0,0 +1 @@ +A DELETE led by WITH, or whose only WHERE sat in a subquery, ran without the dangerous query prompt. diff --git a/apple/changelog.d/3700.fixed.md b/apple/changelog.d/3700.fixed.md new file mode 100644 index 0000000000..22abd531ef --- /dev/null +++ b/apple/changelog.d/3700.fixed.md @@ -0,0 +1 @@ +Saving an edit on SQL Server, Snowflake, Trino or Teradata from a result missing a key column could change other rows. From 9e29ce35b6dd4d7a72487e5c7b53e5c3cf182423 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 20 Oct 2026 21:13:33 +0700 Subject: [PATCH 3/3] fix(probes): declare the probe's locals before assigning them --- apple/scripts/probes/check-mysql-enum-set-save.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apple/scripts/probes/check-mysql-enum-set-save.sh b/apple/scripts/probes/check-mysql-enum-set-save.sh index 48f9b40368..d4594b02bd 100755 --- a/apple/scripts/probes/check-mysql-enum-set-save.sh +++ b/apple/scripts/probes/check-mysql-enum-set-save.sh @@ -125,8 +125,8 @@ keyless_case() { update_using="$update_using, $variable" delete_using="${delete_using:+$delete_using, }$variable" done - local prefix="SET @n = 'large', @oe = $(bind "$enum_value"), @os = $(bind "$set_value");" - local updated deleted + local prefix updated deleted + prefix="SET @n = 'large', @oe = $(bind "$enum_value"), @os = $(bind "$set_value");" updated="$(run "$prefix PREPARE w FROM 'UPDATE \`keyless\` SET \`e\` = ? WHERE $e_match AND $s_match'; EXECUTE w USING $update_using; SELECT ROW_COUNT();")"