diff --git a/lib/core/database/sql_table_target_extractor.dart b/lib/core/database/sql_table_target_extractor.dart index 4ea22412..72f6e783 100644 --- a/lib/core/database/sql_table_target_extractor.dart +++ b/lib/core/database/sql_table_target_extractor.dart @@ -122,7 +122,9 @@ bool _startsWithKeyword(String sql, String keyword) { /// Whether SQL-grid Save may run for this result set. /// /// Requires a simple single-table SELECT, a non-empty primary key, and every -/// PK column present in [resultColumns] (so WHERE can address the row). +/// PK column present in [resultColumns] (so WHERE can address the row) *as a +/// column of the table*: `SELECT number AS id ...` has a column called `id` +/// that is not the key, and an UPDATE keyed on it would change another row. bool sqlResultGridSaveEnabled({ required String? sql, required List resultColumns, @@ -133,7 +135,76 @@ bool sqlResultGridSaveEnabled({ } if (primaryKeys.isEmpty) return false; final cols = resultColumns.toSet(); - return primaryKeys.every(cols.contains); + if (!primaryKeys.every(cols.contains)) return false; + final direct = _directSelectColumns(sql); + if (direct == null) return true; // `SELECT *`: every column is the table's. + final names = {for (final n in direct) n.toLowerCase()}; + return primaryKeys.every((pk) => names.contains(pk.toLowerCase())); +} + +const _selectIdent = r'(?:"[^"]+"|`[^`]+`|\[[^\]]+\]|[\p{L}\p{N}_$]+)'; +final _plainSelectItem = RegExp( + '^(?:$_selectIdent\\s*\\.\\s*)*($_selectIdent)(?:\\s+(?:as\\s+)?($_selectIdent))?\$', + caseSensitive: false, + unicode: true, +); + +String _unquoteIdent(String s) { + if (s.length >= 2 && + ((s.startsWith('"') && s.endsWith('"')) || + (s.startsWith('`') && s.endsWith('`')) || + (s.startsWith('[') && s.endsWith(']')))) { + return s.substring(1, s.length - 1); + } + return s; +} + +/// The output names of the select items of [sql] that are a plain column of +/// the table, unrenamed (`id`, `t.id`, `"id"`, `id AS id`), or null when the +/// list holds a `*` (every column of the table, as is). Items that are an +/// expression or an alias of another column are left out. +List? _directSelectColumns(String sql) { + final text = sql.trim(); + final selectAt = _skipTrivia(text, 0) + 'select'.length; + final fromAt = _indexOfTopLevelKeyword(text, 'from'); + if (fromAt < 0 || fromAt <= selectAt) return null; + var list = text.substring(selectAt, fromAt).trim(); + final quantifier = RegExp(r'^(?:distinct|all)\s+', caseSensitive: false); + list = list.replaceFirst(quantifier, ''); + + final items = []; + var depth = 0; + String? quote; + var start = 0; + for (var i = 0; i < list.length; i++) { + final c = list[i]; + if (quote != null) { + if (c == quote) quote = null; + } else if (c == "'" || c == '"' || c == '`') { + quote = c; + } else if (c == '(') { + depth++; + } else if (c == ')') { + if (depth > 0) depth--; + } else if (c == ',' && depth == 0) { + items.add(list.substring(start, i).trim()); + start = i + 1; + } + } + items.add(list.substring(start).trim()); + + final direct = []; + for (final item in items) { + if (item == '*' || item.endsWith('.*')) return null; + final m = _plainSelectItem.firstMatch(item); + if (m == null) continue; + final column = _unquoteIdent(m.group(1)!); + final alias = m.group(2); + if (alias == null || _unquoteIdent(alias).toLowerCase() == column.toLowerCase()) { + direct.add(column); + } + } + return direct; } int _indexOfTopLevelKeyword(String sql, String keyword) { diff --git a/lib/core/database/table_mutation_engine.dart b/lib/core/database/table_mutation_engine.dart index bbeb4c0c..5245de33 100644 --- a/lib/core/database/table_mutation_engine.dart +++ b/lib/core/database/table_mutation_engine.dart @@ -472,7 +472,9 @@ abstract final class TableMutationEngine { required List primaryKeys, }) { if (primaryKeys.isEmpty) return false; - return primaryKeys.any((pk) { + // Every key column: a composite key with one column missing cannot + // address a single row. + return primaryKeys.every((pk) { if (columns.contains(pk)) return true; final normPk = normalizeIdentifier(pk); return columns.any((c) => normalizeIdentifier(c) == normPk); diff --git a/test/core/database/sql_table_target_extractor_test.dart b/test/core/database/sql_table_target_extractor_test.dart index 69413e96..f68918d0 100644 --- a/test/core/database/sql_table_target_extractor_test.dart +++ b/test/core/database/sql_table_target_extractor_test.dart @@ -131,6 +131,66 @@ void main() { ); }); + test('false when the key name is an alias or an expression (#1772)', () { + for (final sql in [ + 'SELECT number AS id, name FROM t', + 'SELECT number id, name FROM t', + 'SELECT id + 0 AS id, name FROM t', + 'SELECT CAST(number AS int) AS id, name FROM t', + ]) { + expect( + sqlResultGridSaveEnabled( + sql: sql, + resultColumns: ['id', 'name'], + primaryKeys: ['id'], + ), + isFalse, + reason: sql, + ); + } + }); + + test('true when the key is selected as its own column (#1772)', () { + for (final sql in [ + 'SELECT id, name FROM t', + 'SELECT t.id, t.name FROM t', + 'SELECT "id", name FROM t', + 'SELECT id AS id, name FROM t', + 'SELECT DISTINCT id, name FROM t', + 'SELECT t.*, 1 AS extra FROM t', + 'SELECT ID, name FROM t', + ]) { + expect( + sqlResultGridSaveEnabled( + sql: sql, + resultColumns: ['id', 'name'], + primaryKeys: ['id'], + ), + isTrue, + reason: sql, + ); + } + }); + + test('a composite key needs every column (#1772)', () { + expect( + sqlResultGridSaveEnabled( + sql: 'SELECT a, name FROM t', + resultColumns: ['a', 'name'], + primaryKeys: ['a', 'b'], + ), + isFalse, + ); + expect( + sqlResultGridSaveEnabled( + sql: 'SELECT a, b, name FROM t', + resultColumns: ['a', 'b', 'name'], + primaryKeys: ['a', 'b'], + ), + isTrue, + ); + }); + test('false for JOIN and comma FROM', () { expect( sqlResultGridSaveEnabled(