vincent-herlemont / native_db

Drop-in embedded database in Rust
MIT License
433 stars 17 forks source link

Migration fails when another model with a different id exists #159

Closed croissong closed 3 months ago

croissong commented 3 months ago

The following change to the migrate test illustrates the issue. With the additional model it throws an error:

Impossible to migrate the table 1_2_generate_my_primary_key because the table 1_1_generate_my_primary_key has data
diff --git a/tests/migrate/only_primary_key.rs b/tests/migrate/only_primary_key.rs
index 36695d4..33c83b6 100644
--- a/tests/migrate/only_primary_key.rs
+++ b/tests/migrate/only_primary_key.rs
@@ -51,13 +51,27 @@ impl ItemV2 {
     }
 }

+#[derive(Serialize, Deserialize, Eq, PartialEq, Debug, Clone)]
+#[native_model(id = 2, version = 1)]
+#[native_db]
+struct Item2 {
+    #[primary_key]
+    id: u32,
+}
+
 #[test]
 fn test_migrate() {
     let tf = TmpFs::new().unwrap();
     let mut builder = DatabaseBuilder::new();
     builder.define::<ItemV1>().unwrap();
+    builder.define::<Item2>().unwrap();
     let db = builder.create(tf.path("test").as_std_path()).unwrap();

+    let item_2 = Item2 { id: 1 };
+    let rw_txn = db.rw_transaction().unwrap();
+    rw_txn.insert(item_2).unwrap();
+    rw_txn.commit().unwrap();
+
     let item = ItemV1 {
         id: 1,
         name: "test".to_string(),
@@ -83,6 +97,7 @@ fn test_migrate() {
     let mut builder = DatabaseBuilder::new();
     builder.define::<ItemV1>().unwrap();
     builder.define::<ItemV2>().unwrap();
+    builder.define::<Item2>().unwrap();
     let db = builder.create(tf.path("test").as_std_path()).unwrap();

     let rw = db.rw_transaction().unwrap();
croissong commented 3 months ago

Here's my attempt at a simple fix by filtering the self.primary_table_definitions on the native_model_id: #160 - pls lmk what you think :)

I added a separate test case for it, which might be overkill though...

vincent-herlemont commented 3 months ago

@croissong Thank you very much for discovering this bug and for the pull request. I will look into it.

I added a separate test case for it, which might be overkill though...

It's prefect 👌

vincent-herlemont commented 3 months ago

:tada: This issue has been resolved in version 0.7.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: