Only drop triggers during migration, instead of the views.

Author: rkistner

crates/core/src/migrations.rs

@@ -172,21 +172,28 @@ VALUES(4,
     }
 
     if current_version < 5 && target_version >= 5 {
-        // Start by dropping all existing views and triggers (but not tables).
+        // Start by dropping all triggers on views (but not views tables).
         // This is because the triggers are restructured in this version, and
         // need to be re-created from scratch. Not dropping them can make it
         // refer to tables or columns not existing anymore, which can case
         // issues later on.
-        // The same applies for the down migration.
+        //
+        // Similarly, dropping the views themselves can cause issues with
+        // user-defined triggers that refer to them.
+        //
+        // The same applies for the down migration, except there we do drop
+        // the views, since we cannot use the `powersync_views` view.
+        // Down migrations are less common, so we're okay about that breaking
+        // in some cases.
 
         // language=SQLite
         local_db
           .exec_safe(
               "\
-SELECT powersync_drop_view(view.name)
-FROM sqlite_master view
-WHERE view.type = 'view'
-  AND view.sql GLOB  '*-- powersync-auto-generated';
+UPDATE powersync_views SET
+        delete_trigger_sql = '',
+        update_trigger_sql = '',
+        insert_trigger_sql = '';
 
 ALTER TABLE ps_buckets RENAME TO ps_buckets_old;
 ALTER TABLE ps_oplog RENAME TO ps_oplog_old;

dart/test/migration_test.dart

@@ -211,5 +211,40 @@ void main() {
       final data2 = getData(db);
       expect(data2, equals(fix035.dataFixed.trim()));
     });
+
+    /// Here we start with a schema from fixtures on db version 3
+    test('schema 3 -> 5 with custom triggers', () async {
+      db.execute(fixtures.expectedState[3]!);
+      db.execute(fixtures.schema3);
+      // This is a contrived example, but gives us a trigger
+      // that references the "lists" view, affecting migrations.
+      db.execute('''
+create trigger t1 after delete on ps_data__lists begin
+  insert into lists(id, description) values(OLD.id, 'deleted');
+end''');
+
+      var tableSchema = {
+        'tables': [
+          {
+            'name': 'lists',
+            'columns': [
+              {'name': 'description', 'type': 'TEXT'}
+            ]
+          }
+        ]
+      };
+      db.select('select powersync_init()');
+      db.select(
+          'select powersync_replace_schema(?)', [jsonEncode(tableSchema)]);
+
+      final schema = getSchema(db);
+      final expected =
+          '''${fixtures.finalState.replaceAll(RegExp(r';INSERT INTO ps_migration.*'), '').trim()}
+${fixtures.schema5.trim()}
+;CREATE TRIGGER t1 after delete on ps_data__lists begin
+  insert into lists(id, description) values(OLD.id, \'deleted\');
+end''';
+      expect(schema, equals(expected));
+    });
   });
 }