No SqliteException thrown if when fetching data from non existent column in test mode

[noxasch]

Describe the bug

I don't think this is a critical as it only happen in during test mode.

When running migration test, fetching data from non existent column return empty array.

Expected:
throw SqliteException as in debug and release mode

[simolus3]

Thanks for the report - this sounds pretty bizarre though, I don't think we're catching those errors anywhere and I don't think sqlite3 will just accept those queries either.

Do you have a reduced example that reproduces this (e.g. the tables before and after, your migration and how the test looks like)?

[noxasch]

@simolus3 here some reduced example with the migration and test.

What I'm trying to do with the migration is since sqlite doesn't support IF NOT EXIST and trying not to read the schema since it will require me to write another model just for the migration.

My test is very simple, here are some example and the workaround that I'm currently use to make sure the test passed on CI.

model changes

// v1
class Accounts extends Table {
  IntColumn get id => integer().autoIncrement()();
  TextColumn get name => text().withLength(max: 250)();
  DateTimeColumn get createdAt => dateTime().clientDefault(() => clock.now())();
  DateTimeColumn get updatedAt => dateTime().clientDefault(() => clock.now())();
}

// v2
class Accounts extends Table {
  IntColumn get id => integer().autoIncrement()();
  TextColumn get name => text().withLength(max: 250)();
  DateTimeColumn get createdAt => dateTime().clientDefault(() => clock.now())();
  DateTimeColumn get updatedAt => dateTime().clientDefault(() => clock.now())();
  BoolColumn get isDefault => boolean().withDefault(const Constant(false))();
}

migration

MigrationStrategy get migration {
    return MigrationStrategy(
      onUpgrade: (Migrator m, int from, int to) async {
        await customStatement('PRAGMA foreign_keys = OFF');
      
        if (from == 1 && to == 2) {
          try {
            final rows = await customSelect(
              'SELECT "is_default" FROM accounts LIMIT 1;',
              readsFrom: {accounts},
            ).get();

            // WORKAROUND: no error is thrown in migration test
            final bool kTestMode =
                Platform.environment.containsKey('FLUTTER_TEST');
            if (kTestMode && rows.isEmpty) {
              await m.addColumn(accounts, accounts.isDefault);
              await customStatement('UPDATE accounts SET is_default = 0;');
            }
            // WORKAROUND END
          } on SqliteException catch (e) {
            if (e.message.contains('no such column')) {
              await m.addColumn(accounts, accounts.isDefault);
              await customStatement('UPDATE accounts SET is_default = 0;');
            }
          }
        }
      },
      beforeOpen: (OpeningDetails details) async {
        await customStatement('PRAGMA foreign_keys = ON');
      },
    );
  }

schema validation test

group('Schema Validation', () {
    late SchemaVerifier verifier;

   test('upgrade from v1 to v2', () async {
      final connection = await verifier.startAt(1);
      final db = AppDatabase.connect(connection);
      addTearDown(db.close);
      await verifier.migrateAndValidate(db, 2);
  });
}

[noxasch]

just to add after I encounter issue that depending on SqliteException is not that reliable in this case, I moved to following strategy instead.

if (from == 1 && to == 2) {
     final rows = await customSelect(
        'SELECT * FROM pragma_table_info(\'accounts\') WHERE name=\'is_default\';',
         readsFrom: {accounts},
      ).get();

     if (rows.isEmpty) {
        await m.addColumn(accounts, accounts.isDefault);
        await customStatement('UPDATE accounts SET is_default = 0;');
      }
}

[simolus3]

Argh damn. We have been tricked by the horrible double-quoted strings misfeature. I should know better, but this still took me by surprise.
When you write SELECT "is_default" FROM accounts LIMIT 1;, some sqlite3 builds see that there is no is_default column which you've referenced and then think that you've obviously meant to select a string literal from the table instead of using the reference syntax to, well, reference a column.

The builds provided by sqlite3_flutter_libs don't do this, but migration tests are unit tests and they use the sqlite3 build from your system. You can fix this by applying a setup callback when creating the SchemaVerifier:

verifier = SchemaVerifier(schema.GeneratedHelper(),
    setup: (raw) => raw.config.doubleQuotedStringLiterals = false);

I will also apply this option by default since it's super confusing.

And one note that is unrelated but still good to be aware of: You won't get a SqliteException thrown when using NativeDatabase.createInBackground() - that would be a DriftRemoteException in that case. Something to be aware of if you intend to migrate to a background isolate.

Finally, ideally you shouldn't have to run schema checks at runtime since the migration isn't supposed to run when the column is already there.