Merge pull request #48 from powersync-ja/fix-triggers-migration

Fix view migration issue

Author: rkistner

.github/workflows/macos.yml

@@ -21,7 +21,7 @@ jobs:
 
   build_macOS_x64:
     name: Building macOS x64
-    runs-on: macos-12
+    runs-on: macos-14
     steps:
       - uses: actions/checkout@v3
         with:

.github/workflows/release.yml

@@ -244,7 +244,7 @@ jobs:
   publish_macOS_x64:
     name: Publish macOS x64
     needs: [draft_release]
-    runs-on: macos-12
+    runs-on: macos-14
     steps:
       - uses: actions/checkout@v3
         with:

Cargo.lock

@@ -24,7 +24,7 @@ inherits = "release"
 inherits = "wasm"
 
 [workspace.package]
-version = "0.3.6"
+version = "0.3.7"
 edition = "2021"
 authors = ["JourneyApps"]
 keywords = ["sqlite", "powersync"]

Cargo.toml

@@ -6,7 +6,7 @@ plugins {
 }
 
 group = "co.powersync"
-version = "0.3.6"
+version = "0.3.7"
 description = "PowerSync Core SQLite Extension"
 
 repositories {

android/build.gradle.kts

@@ -2,5 +2,5 @@
   "name": "powersync_sqlite_core",
   "schema_version": 2,
   "dependencies": [],
-  "version": "0.3.6"
+  "version": "0.3.7"
 }

android/src/prefab/prefab.json

@@ -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;

crates/core/src/migrations.rs

@@ -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));
+    });
   });
 }

dart/test/migration_test.dart

@@ -1,6 +1,6 @@
 Pod::Spec.new do |s|
   s.name             = 'powersync-sqlite-core'
-  s.version          = '0.3.6'
+  s.version          = '0.3.7'
   s.summary          = 'PowerSync SQLite Extension'
   s.description      = <<-DESC
 PowerSync extension for SQLite.

powersync-sqlite-core.podspec

@@ -28,9 +28,9 @@ function createXcframework() {
   <key>MinimumOSVersion</key>
   <string>11.0</string>
   <key>CFBundleVersion</key>
-  <string>0.3.6</string>
+  <string>0.3.7</string>
   <key>CFBundleShortVersionString</key>
-  <string>0.3.6</string>
+  <string>0.3.7</string>
 </dict>
 </plist>
 EOF