Merge pull request #2308 from davidmartos96/pg_improvements

Remove hackyvariables from Postgres tests

drift/lib/src/runtime/query_builder/expressions/variables.dart

@@ -74,15 +74,23 @@ class Variable<T extends Object> extends Expression<T> {
     var explicitStart = context.explicitVariableIndex;
 
     var mark = '?';
+    var suffix = '';
     if (context.dialect == SqlDialect.postgres) {
       explicitStart = 1;
       mark = '@';
+
+      if (value is List<int>) {
+        // 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),

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;
@@ -58,9 +58,7 @@ class _PgDelegate extends DatabaseDelegate {
       final stmt = statements.statements[row.statementIndex];
       final args = row.arguments;
 
-      await _ec.execute(stmt,
+      await _ec.execute(stmt, substitutionValues: _convertArgs(args));
-          substitutionValues: args.asMap().map((key, value) =>
-              MapEntry((key + 1).toString(), _convertValue(value))));
     }
 
     return Future.value();
@@ -72,9 +70,7 @@ class _PgDelegate extends DatabaseDelegate {
     if (args.isEmpty) {
       return _ec.execute(statement);
     } else {
-      return _ec.execute(statement,
+      return _ec.execute(statement, substitutionValues: _convertArgs(args));
-          substitutionValues: args.asMap().map((key, value) =>
-              MapEntry((key + 1).toString(), _convertValue(value))));
     }
   }
 
@@ -90,9 +86,10 @@ class _PgDelegate extends DatabaseDelegate {
     if (args.isEmpty) {
       result = await _ec.query(statement);
     } else {
-      result = await _ec.query(statement,
+      result = await _ec.query(
-          substitutionValues: args.asMap().map((key, value) =>
+        statement,
-              MapEntry((key + 1).toString(), _convertValue(value))));
+        substitutionValues: _convertArgs(args),
+      );
     }
     return result.firstOrNull?[0] as int? ?? 0;
   }
@@ -105,9 +102,10 @@ class _PgDelegate extends DatabaseDelegate {
   @override
   Future<QueryResult> runSelect(String statement, List<Object?> args) async {
     await _ensureOpen();
-    final result = await _ec.query(statement,
+    final result = await _ec.query(
-        substitutionValues: args.asMap().map((key, value) =>
+      statement,
-            MapEntry((key + 1).toString(), _convertValue(value))));
+      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<String, dynamic> _convertArgs(List<Object?> args) {
+    return args.asMap().map(
+          (key, value) => MapEntry(
+            (key + 1).toString(),
+            _convertValue(value),
+          ),
+        );
+  }
 }
 
 class _PgVersionDelegate extends DynamicVersionDelegate {
@@ -154,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)));
-  }
-}

extras/drift_postgres/test/drift_postgres_test.dart

@@ -7,7 +7,7 @@ class PgExecutor extends TestExecutor {
   bool get supportsReturning => true;
 
   @override
-  bool get hackyVariables => true;
+  bool get supportsNestedTransactions => true;
 
   @override
   DatabaseConnection createConnection() {

extras/integration_tests/drift_testcases/lib/database/database.dart

@@ -69,7 +69,7 @@ class PreferenceConverter extends NullAwareTypeConverter<Preferences, String> {
         '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".**

extras/integration_tests/drift_testcases/lib/database/database.g.dart

@@ -555,7 +555,7 @@ abstract class _$Database extends GeneratedDatabase {
 
   Selectable<int> 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<int>(user)
         ],

extras/integration_tests/drift_testcases/lib/suite/crud_tests.dart

@@ -119,11 +119,29 @@ void crudTests(TestExecutor executor) {
     tearDown(() => executor.clearDatabaseAndClose(database));
 
     Future<T?> evaluate<T extends Object>(Expression<T> expr) async {
+      late final Expression<T> 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<T>();
+      } 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<T?>(),
+        reason:
+            "Type of the input argument does not match the returned column value",
+      );
+      return columnValue;
     }
 
     test('null', () {