From fef15389e6e34da1066612fef15ff890c6c22eef Mon Sep 17 00:00:00 2001 From: David Martos Date: Thu, 9 Feb 2023 00:23:29 +0100 Subject: [PATCH 1/3] Remove hackyvariables from Postgres tests --- .../query_builder/expressions/variables.dart | 10 +++++- .../drift_postgres/lib/src/pg_database.dart | 31 ++++++++++++------- .../test/drift_postgres_test.dart | 3 -- .../drift_testcases/lib/suite/crud_tests.dart | 22 +++++++++++-- 4 files changed, 48 insertions(+), 18 deletions(-) diff --git a/drift/lib/src/runtime/query_builder/expressions/variables.dart b/drift/lib/src/runtime/query_builder/expressions/variables.dart index a03815c0..d3b4cf41 100644 --- a/drift/lib/src/runtime/query_builder/expressions/variables.dart +++ b/drift/lib/src/runtime/query_builder/expressions/variables.dart @@ -74,15 +74,23 @@ class Variable extends Expression { var explicitStart = context.explicitVariableIndex; var mark = '?'; + var suffix = ''; if (context.dialect == SqlDialect.postgres) { explicitStart = 1; mark = '@'; + + if (value is List) { + // We need to explicitly bind the variable as byte array. Otherwise + // a Uint8List like [1,2,3] would bind to "{1,2,3}" as bytes + suffix += ":bytea"; + } } if (explicitStart != null) { context.buffer ..write(mark) - ..write(explicitStart + context.amountOfVariables); + ..write(explicitStart + context.amountOfVariables) + ..write(suffix); context.introduceVariable( this, mapToSimpleValue(context), diff --git a/extras/drift_postgres/lib/src/pg_database.dart b/extras/drift_postgres/lib/src/pg_database.dart index 64a9b1cd..99ee8ae0 100644 --- a/extras/drift_postgres/lib/src/pg_database.dart +++ b/extras/drift_postgres/lib/src/pg_database.dart @@ -58,9 +58,7 @@ class _PgDelegate extends DatabaseDelegate { final stmt = statements.statements[row.statementIndex]; final args = row.arguments; - await _ec.execute(stmt, - substitutionValues: args.asMap().map((key, value) => - MapEntry((key + 1).toString(), _convertValue(value)))); + await _ec.execute(stmt, substitutionValues: _convertArgs(args)); } return Future.value(); @@ -72,9 +70,7 @@ class _PgDelegate extends DatabaseDelegate { if (args.isEmpty) { return _ec.execute(statement); } else { - return _ec.execute(statement, - substitutionValues: args.asMap().map((key, value) => - MapEntry((key + 1).toString(), _convertValue(value)))); + return _ec.execute(statement, substitutionValues: _convertArgs(args)); } } @@ -90,9 +86,10 @@ class _PgDelegate extends DatabaseDelegate { if (args.isEmpty) { result = await _ec.query(statement); } else { - result = await _ec.query(statement, - substitutionValues: args.asMap().map((key, value) => - MapEntry((key + 1).toString(), _convertValue(value)))); + result = await _ec.query( + statement, + substitutionValues: _convertArgs(args), + ); } return result.firstOrNull?[0] as int? ?? 0; } @@ -105,9 +102,10 @@ class _PgDelegate extends DatabaseDelegate { @override Future runSelect(String statement, List args) async { await _ensureOpen(); - final result = await _ec.query(statement, - substitutionValues: args.asMap().map((key, value) => - MapEntry((key + 1).toString(), _convertValue(value)))); + final result = await _ec.query( + statement, + substitutionValues: _convertArgs(args), + ); return Future.value(QueryResult.fromRows( result.map((e) => e.toColumnMap()).toList(growable: false))); @@ -126,6 +124,15 @@ class _PgDelegate extends DatabaseDelegate { } return value; } + + Map _convertArgs(List args) { + return args.asMap().map( + (key, value) => MapEntry( + (key + 1).toString(), + _convertValue(value), + ), + ); + } } class _PgVersionDelegate extends DynamicVersionDelegate { diff --git a/extras/drift_postgres/test/drift_postgres_test.dart b/extras/drift_postgres/test/drift_postgres_test.dart index d0f3685b..bdbeaed4 100644 --- a/extras/drift_postgres/test/drift_postgres_test.dart +++ b/extras/drift_postgres/test/drift_postgres_test.dart @@ -6,9 +6,6 @@ class PgExecutor extends TestExecutor { @override bool get supportsReturning => true; - @override - bool get hackyVariables => true; - @override DatabaseConnection createConnection() { final pgConnection = PostgreSQLConnection('localhost', 5432, 'postgres', diff --git a/extras/integration_tests/drift_testcases/lib/suite/crud_tests.dart b/extras/integration_tests/drift_testcases/lib/suite/crud_tests.dart index 0650629c..d053665c 100644 --- a/extras/integration_tests/drift_testcases/lib/suite/crud_tests.dart +++ b/extras/integration_tests/drift_testcases/lib/suite/crud_tests.dart @@ -119,11 +119,29 @@ void crudTests(TestExecutor executor) { tearDown(() => executor.clearDatabaseAndClose(database)); Future evaluate(Expression expr) async { + late final Expression effectiveExpr; + if (database.executor.dialect == SqlDialect.postgres) { + // 'SELECT'ing values that don't come from a table return as String + // by default, so we need to explicitly cast it to the expected type + // https://www.postgresql.org/docs/current/typeconv-select.html + effectiveExpr = expr.cast(); + } else { + effectiveExpr = expr; + } + final query = database.selectOnly(database.users) - ..addColumns([expr]) + ..addColumns([effectiveExpr]) ..limit(1); final row = await query.getSingle(); - return row.read(expr); + final columnValue = row.read(effectiveExpr); + + expect( + columnValue, + TypeMatcher(), + reason: + "Type of the input argument does not match the returned column value", + ); + return columnValue; } test('null', () { From bfed38b806ca731ecf743ab7714858453f93f6b9 Mon Sep 17 00:00:00 2001 From: David Martos Date: Thu, 9 Feb 2023 11:02:35 +0100 Subject: [PATCH 2/3] fix query from integration_tests to make it compatible across SQL Dialects --- .../drift_testcases/lib/database/database.dart | 2 +- .../drift_testcases/lib/database/database.g.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/extras/integration_tests/drift_testcases/lib/database/database.dart b/extras/integration_tests/drift_testcases/lib/database/database.dart index 40d1db76..b285842c 100644 --- a/extras/integration_tests/drift_testcases/lib/database/database.dart +++ b/extras/integration_tests/drift_testcases/lib/database/database.dart @@ -69,7 +69,7 @@ class PreferenceConverter extends NullAwareTypeConverter { 'ORDER BY (SELECT COUNT(*) FROM friendships ' 'WHERE first_user = u.id OR second_user = u.id) DESC LIMIT :amount', 'amountOfGoodFriends': 'SELECT COUNT(*) FROM friendships f WHERE ' - 'f.really_good_friends = 1 AND ' + 'f.really_good_friends = TRUE AND ' '(f.first_user = :user OR f.second_user = :user)', 'friendshipsOf': ''' SELECT f.really_good_friends, "user".** diff --git a/extras/integration_tests/drift_testcases/lib/database/database.g.dart b/extras/integration_tests/drift_testcases/lib/database/database.g.dart index b5689ce6..34d935f8 100644 --- a/extras/integration_tests/drift_testcases/lib/database/database.g.dart +++ b/extras/integration_tests/drift_testcases/lib/database/database.g.dart @@ -555,7 +555,7 @@ abstract class _$Database extends GeneratedDatabase { Selectable amountOfGoodFriends(int user) { return customSelect( - 'SELECT COUNT(*) AS _c0 FROM friendships AS f WHERE f.really_good_friends = 1 AND(f.first_user = @1 OR f.second_user = @1)', + 'SELECT COUNT(*) AS _c0 FROM friendships AS f WHERE f.really_good_friends = TRUE AND(f.first_user = @1 OR f.second_user = @1)', variables: [ Variable(user) ], From d6edc787d8571846fe4ec27861939def9e79cf66 Mon Sep 17 00:00:00 2001 From: David Martos Date: Thu, 9 Feb 2023 11:17:03 +0100 Subject: [PATCH 3/3] nested transaction support in PgDatabase --- extras/drift_postgres/lib/src/pg_database.dart | 16 +--------------- .../drift_postgres/test/drift_postgres_test.dart | 3 +++ 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/extras/drift_postgres/lib/src/pg_database.dart b/extras/drift_postgres/lib/src/pg_database.dart index 99ee8ae0..c897fb57 100644 --- a/extras/drift_postgres/lib/src/pg_database.dart +++ b/extras/drift_postgres/lib/src/pg_database.dart @@ -25,7 +25,7 @@ class _PgDelegate extends DatabaseDelegate { final bool closeUnderlyingWhenClosed; @override - TransactionDelegate get transactionDelegate => _PgTransactionDelegate(_db); + TransactionDelegate get transactionDelegate => const NoTransactionDelegate(); @override late DbVersionDelegate versionDelegate; @@ -161,17 +161,3 @@ class _PgVersionDelegate extends DynamicVersionDelegate { substitutionValues: {'1': version}); } } - -class _PgTransactionDelegate extends SupportedTransactionDelegate { - final PostgreSQLConnection _db; - - const _PgTransactionDelegate(this._db); - - @override - bool get managesLockInternally => false; - - @override - Future startTransaction(Future Function(QueryDelegate) run) async { - await _db.transaction((connection) => run(_PgDelegate(_db, connection))); - } -} diff --git a/extras/drift_postgres/test/drift_postgres_test.dart b/extras/drift_postgres/test/drift_postgres_test.dart index bdbeaed4..32d07208 100644 --- a/extras/drift_postgres/test/drift_postgres_test.dart +++ b/extras/drift_postgres/test/drift_postgres_test.dart @@ -6,6 +6,9 @@ class PgExecutor extends TestExecutor { @override bool get supportsReturning => true; + @override + bool get supportsNestedTransactions => true; + @override DatabaseConnection createConnection() { final pgConnection = PostgreSQLConnection('localhost', 5432, 'postgres',