From c3fba11e91c3547d9a51e44ba3981d48b53622f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= <jorgen.edelbo@mongodb.com>
Date: Thu, 2 Jan 2025 17:00:31 +0100
Subject: [PATCH] Do not set primary key column before migration function is
 called

This is fixing a regression introduced by f7c8e97. The change done in
this commit is reverted. Instead the primary key column is set in the
post migration changes.
---
 CHANGELOG.md                            |  4 +-
 src/realm/object-store/object_store.cpp | 11 ++++--
 test/object-store/migrations.cpp        | 50 +++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b560361004..c9b96b1f40 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,8 +5,7 @@
 * None.
 
 ### Fixed
-* <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
-* None.
+* Changing type of primary key column crashes if more than one object ([#8056](https://github.com/realm/realm-core/issues/8056), since v14.13.2)
 
 ### Breaking changes
 * None.
@@ -28,7 +27,6 @@
 
 ### Fixed
 * Migrating primary key to a new type without migration function would cause an assertion to fail. ([#8045](https://github.com/realm/realm-core/issues/8045), since v10.0.0)
-* None.
 
 ### Breaking changes
 * None.
diff --git a/src/realm/object-store/object_store.cpp b/src/realm/object-store/object_store.cpp
index 254cb0b272..ec061d8f3d 100644
--- a/src/realm/object-store/object_store.cpp
+++ b/src/realm/object-store/object_store.cpp
@@ -140,9 +140,7 @@ ColKey add_column(Group& group, Table& table, Property const& property)
     else {
         auto key =
             table.add_column(to_core_type(property.type), property.name, is_nullable(property.type), collection_type);
-        if (property.is_primary)
-            table.set_primary_key_column(key); // You can end here if this is a migration
-        else if (property.requires_index())
+        if (property.requires_index())
             table.add_search_index(key);
         else if (property.requires_fulltext_index())
             table.add_fulltext_index(key);
@@ -833,7 +831,12 @@ static void apply_post_migration_changes(Group& group, std::vector<SchemaChange>
                                             handle_backlinks_automatically);
         }
         void operator()(RemoveTable) {}
-        void operator()(ChangePropertyType) {}
+        void operator()(ChangePropertyType op)
+        {
+            if (op.new_property->is_primary) {
+                set_primary_key(table(op.object), op.new_property);
+            }
+        }
         void operator()(MakePropertyNullable) {}
         void operator()(MakePropertyRequired) {}
         void operator()(AddProperty) {}
diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp
index dd4968fc21..b634c6def6 100644
--- a/test/object-store/migrations.cpp
+++ b/test/object-store/migrations.cpp
@@ -1137,6 +1137,55 @@ TEST_CASE("migration: Automatic", "[migration]") {
         });
     }
 
+    SECTION("change primary key from int to string") {
+        using namespace std::string_literals;
+        Schema schema{{"Foo",
+                       {
+                           {"_id", PropertyType::Int, Property::IsPrimary{true}},
+                           {"value", PropertyType::String},
+                       }}};
+        Schema schema2{{"Foo",
+                        {
+                            {"_id", PropertyType::String, Property::IsPrimary{true}},
+                            {"value", PropertyType::String},
+                        }}};
+        TestFile config;
+        config.schema_mode = SchemaMode::Automatic;
+        config.schema = schema;
+        config.schema_version = 1;
+        {
+            auto realm = Realm::get_shared_realm(config);
+
+            CppContext ctx(realm);
+            std::any value1 = AnyDict{
+                {"_id", INT64_C(1)},
+                {"value", "foo"s},
+            };
+            std::any value2 = AnyDict{
+                {"_id", INT64_C(2)},
+                {"value", "bar"s},
+            };
+            realm->begin_transaction();
+            auto s = realm->schema().find("Foo");
+            Object::create(ctx, realm, *s, value1);
+            Object::create(ctx, realm, *s, value2);
+            realm->commit_transaction();
+        }
+        config.schema = schema2;
+        config.schema_version = 2;
+        config.migration_function = [](SharedRealm old_realm, SharedRealm realm, Schema&) {
+            Results r{Class(old_realm, &*old_realm->schema().find("Foo"))};
+            auto sz = r.size();
+            auto t = ObjectStore::table_for_object_type(realm->read_group(), "Foo");
+            for (size_t i = 0; i < sz; i++) {
+                Obj o = r.get(i);
+                auto new_obj = t->get_object(o.get_key());
+                new_obj.set("_id", util::to_string(o.get<Int>("_id")));
+            }
+        };
+        auto realm = Realm::get_shared_realm(config);
+    }
+
     SECTION("change primary key from string to UUID without migration function") {
         using namespace std::string_literals;
         Schema schema{{"Foo",
@@ -1153,6 +1202,7 @@ TEST_CASE("migration: Automatic", "[migration]") {
         auto realm = Realm::get_shared_realm(config);
         realm->update_schema(schema2, 2);
 
+        // Make sure you can actually create an object with UUID as primary key
         CppContext ctx(realm);
         std::any values = AnyDict{
             {"_id", UUID("3b241101-0000-0000-0000-4136c566a964"s)},