Skip to content

Commit af493ff

Browse files
authored
fix: Prevent exception from id column in update queries. (serverpod#3327)
1 parent f9e1f53 commit af493ff

File tree

2 files changed

+61
-13
lines changed

2 files changed

+61
-13
lines changed

packages/serverpod/lib/src/database/adapters/postgres/database_connection.dart

+12-13
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,11 @@ class DatabaseConnection {
183183

184184
var table = rows.first.table;
185185

186-
var selectedColumns = columns ?? table.columns;
186+
var selectedColumns = (columns ?? table.columns).toSet();
187187

188188
if (columns != null) {
189-
_validateColumnsExists(columns, table);
190-
selectedColumns = [table.id, ...columns];
189+
_validateColumnsExists(selectedColumns, table.columns.toSet());
190+
selectedColumns.add(table.id);
191191
}
192192

193193
var selectedColumnNames = selectedColumns.map((e) => e.columnName);
@@ -198,7 +198,6 @@ class DatabaseConnection {
198198
var values = _createQueryValueList(rows, selectedColumns);
199199

200200
var setColumns = selectedColumnNames
201-
.where((columnName) => columnName != 'id')
202201
.map((columnName) => '"$columnName" = data."$columnName"')
203202
.join(', ');
204203

@@ -672,15 +671,15 @@ class DatabaseConnection {
672671
return resolvedListRelations;
673672
}
674673

675-
void _validateColumnsExists(List<Column> columns, Table table) {
676-
for (var column in columns) {
677-
if (!table.columns.any((c) => c.columnName == column.columnName)) {
678-
throw ArgumentError.value(
679-
column,
680-
column.columnName,
681-
'does not exist in row',
682-
);
683-
}
674+
void _validateColumnsExists(Set<Column> columns, Set<Column> tableColumns) {
675+
var additionalColumns = columns.difference(tableColumns);
676+
677+
if (additionalColumns.isNotEmpty) {
678+
throw ArgumentError.value(
679+
additionalColumns.toList().toString(),
680+
'columns',
681+
'Columns do not exist in table',
682+
);
684683
}
685684
}
686685

tests/serverpod_test_server/test_integration/database_operations/crud/update_test.dart

+49
Original file line numberDiff line numberDiff line change
@@ -782,5 +782,54 @@ void main() async {
782782

783783
expect(updated.first.anEnum, equals(TestEnum.one));
784784
});
785+
786+
test(
787+
'when listing id column in an update query of a row then update completes successfully.',
788+
() async {
789+
expect(
790+
Types.db.updateRow(session, type, columns: (t) => [t.id]),
791+
completes,
792+
);
793+
});
794+
});
795+
796+
group('Given empty model in database', () {
797+
late EmptyModelWithTable model;
798+
setUp(() async {
799+
model = await EmptyModelWithTable.db
800+
.insertRow(session, EmptyModelWithTable());
801+
});
802+
803+
tearDown(() async {
804+
await EmptyModelWithTable.db
805+
.deleteWhere(session, where: (t) => Constant.bool(true));
806+
});
807+
808+
test('when model is updated then update completes', () async {
809+
expect(
810+
EmptyModelWithTable.db.updateRow(session, model),
811+
completes,
812+
);
813+
});
814+
815+
test('when with columns from different model then error is thrown',
816+
() async {
817+
var invalidColumns = [SimpleData.t.num, Types.t.anInt];
818+
expect(
819+
EmptyModelWithTable.db
820+
.updateRow(session, model, columns: (t) => invalidColumns),
821+
throwsA(
822+
isA<ArgumentError>()
823+
.having(
824+
(e) => e.message,
825+
'message',
826+
equals('Columns do not exist in table'),
827+
)
828+
.having((e) => e.name, 'name', 'columns')
829+
.having((e) => e.invalidValue, 'invalidValue',
830+
invalidColumns.toString()),
831+
),
832+
);
833+
});
785834
});
786835
}

0 commit comments

Comments
 (0)