Throw an error when updating primary key field

[rorbech]

Expected results

I don't think it should be possible to update a primary key field of an object. So I would expect the C-API to throw an error if doing so. If it should be possible to update it, I would at least expect a check that you are not creating duplicate primary keys when setting it.

Actual Results

In realm-kotlin I am able to define a schema with a primary key and after creating of an instance I am able to update the primary key field even setting it to the same primary key value of an already existing object.

I#### Steps & Code to Reproduce

class PrimaryKeyString : RealmObject {
    @PrimaryKey
    var primaryKey: String = Random.nextULong().toString()
}

realm.writeBlocking {
    val first = copyToRealm(PrimaryKeyString().apply { primaryKey = PRIMARY_KEY })
    val second = copyToRealm(PrimaryKeyString().apply { primaryKey = "Other key" })
    
    // expect update to cause an error
    assertFailsWith<RuntimeException> { 
        second.primaryKey = PRIMARY_KEY
    }
    
    // eliminating the above assert will render the following to pass, which shows that we end up with two objects both with the same primary key value
    val objects = objects(PrimaryKeyString::class)
    assertEquals(2, objects.size)
    val first2 = objects[0]
    val second2 = objects[1]
    assertEquals(objects[0].primaryKey, objects[1].primaryKey)
}

Core version

Core version: 10.8.0

[jedelbo]

This cannot be done in a non-breaking way as we actually allow primary key values to be changed during migration to a new schema. As we at the Core level does not know when we are at this stage, then we don't know if it is legal or not.

[rorbech]

It probably makes sense to allow during migration, but feels very dangerous that it is allowed for normal writes without a duplicate violation check when committing. To me it really sounds like "preventing primary key updates" for normal writes should be enforced by core semantics instead of as implementation details throughout the various SDKs.

[nirinchev]

Sounds like something ideally changed as part of the public API rework when we have something wrapping the entire object interface.

[cmelchior]

Hi @jedelbo We agree that it should be possible to have the same primary key during migrations, so this cannot be implemented everywhere. But it seems weird that we are forcing all SDK's similar checks that prevent changing primary key values outside that.

Since ObjectStore knows when the migration callback is being invoked, it should be possible for ObjectStore to enable this behavior whether inside or outside a migration transaction.

But agree with @nirinchev that this probably doesn't have the highest priority, but should be considered as part of the public API revamp.

[sync-by-unito[bot]]

➤ Jørgen Edelbo commented:

Moved to backlog.