Moor: Remove reference to database in executor

Created on 1 Feb 2020  路  7Comments  路  Source: simolus3/moor

QueryExecutors shouldn't have a reference to a GeneratedDatabase. Apart from being unclean design, having to pass all the databases around is annoying when writing new executors.

Instead, we could extract the beforeOpenCallback into a typedef and only pass that to executors. Starting the migration would then happen in GeneratedDatabase and not in an executor. Setting the schema version on a database should also happen explicitly.

Since QueryExecutor.databaseInfo is a public field, this is a breaking change and needs to wait for moor 3.0.

internal

All 7 comments

cc @Mike278: IIRC you used the fake GeneratedDatbase subclass trick to open a raw QueryExecutor. With moor 3.0 I'm removing the QueryExecutor.databaseInfo field. Instead, you would pass a subclass of QueryExecutorUser (GeneratedDatabase will do) to QueryExecutor.ensureOpen. I've also moved QueryExecutor.doWhenOpened to QueryEngine because it can automatically pass the right user onto ensureOpen.

@simolus3 I moved to the current develop to start testing the new version and I now have FK violations in batches. I assume this is related to how/where I enable PRAGMA foreign_keys = ON and it might be related to this change.

I used to do it in the database constructor:

  FooDatabase(QueryExecutor database)
      : super(database
          ..doWhenOpened<void>((db) {
            return db.runCustom('PRAGMA foreign_keys = ON');
          }));

And now I moved this to the MigrationStrategy:

  @override
  MigrationStrategy get migration => MigrationStrategy(
        beforeOpen: (details) async {
          await customStatement('PRAGMA foreign_keys = ON');
        },
      );

What is the difference of these in regard to their order.
My batches run right after the database is created in a test case .

The order should be

  • migrations
  • beforeOpen callback inside a MigrationStrategy
  • doWhenOpened callback

However, there are no guarantees on the order of db.runCustom in doWhenOpened and the first batch run right after creation. When putting the pragma into the beforeOpen callback, it's guaranteed to run before the batch.

Ok so up until now my batch was probably running without FK checks. Now it is running with FK checks and those fail because parts of the batch depends on other parts and this is not support in batches, as far as I have learned in the last hour.

Ouch, moor should probably run batched statements in the order they're issued. At the moment we collect them in a Map<String, List<List<dynamic>> to map from sql statements to the variables. This ensures we don't have to prepare the same statement multiple times, but we loose the order. I'll take a look at that now.

Should be fixed with 821d1620cd67c41578ce8f10a30c953e558244c7

My tests are green, nice work thanks!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

tony123S picture tony123S  路  4Comments

kira1752 picture kira1752  路  3Comments

tony123S picture tony123S  路  4Comments

emshack picture emshack  路  3Comments

Holofox picture Holofox  路  4Comments