Primary Keys in Core

[simonask]

PROPOSAL -- very open for discussion.

This does two things:

1. Removes all previous support for primary keys in Core.
2. Introduces a new operation, "SetUnique", which gives an error if the incoming value already exists in the column.

To implement primary keys on top of this, the binding must call only the set_unique operations (and never plain set) on a column. An extra consequence of this is that primary key constraints are only checked when set_unique is called, and not, for instance, when new rows are inserted. This replicates the current behavior of the mechanism used in the Cocoa binding (see RLMSetValueUnique).

Rationale

• We need Core-level support for primary keys in order to support them in Sync. Since updating a value in a primary key column has different side-effects than updating a value in a non-PK column, the PK nature of the update operation must be encoded in the transaction log, so we can implement different merge rules for PK updates.
• This retains the current implementation model for primary keys. Bindings still use the pk metadata table, and Core still knows nothing about which columns are primary key columns (no attribute or similar). It is solely up to the binding / object store to ensure that primary key columns are treated as such, as it is today. This has the additional benefit of not requiring a new method of creating objects, i.e. this retains the insert-empty-row-then-set-values mode.
• As discussed in Primary key support  #831, different semantics for primary keys than what users usually expect are required for sync, because we cannot throw a primary key constraint violation during merge in case of a conflict. The proposed semantics is to merge the objects with the conflicting primary keys into one object. However, this means primary keys would behave differently in sync and outside of sync, so there is a proposal to actually make non-sync "primary keys" behave this way as well. This has the implication that changing the PK of an object to the same value as some other object will actually cause one of the objects to be deleted and the properties of the objects to be merged, and all references to the deleted object to be changed to point to the retained object. This behavior is probably extremely confusing for users by itself if we call it "primary keys", so this could be a reason to go for a different naming scheme, in this case "unique".

I have also considered the word "identity" instead of "unique", but "identity" is already a heavily overloaded word... Still, it might be worth considering.

Fixes #831.

\cc @kspangsege

[realm-ci]

Coverage: 30%

Please check your coverage here: https://ci.realm.io/job/core_matrix_pr/1062/buildcommand=jenkins-pipeline-coverage,buildtype=debug,encryption=no,slave=fastlinux/Diff_Coverage/

[realm-ci]

Coverage: 31%

Please check your coverage here: https://ci.realm.io/job/core_matrix_pr/1065/buildcommand=jenkins-pipeline-coverage,buildtype=debug,encryption=no,slave=fastlinux/Diff_Coverage/

[kneth]

@simonask This is a single column uniqueness, right? In general, candidate/primary keys can span multiple columns. To my knowledge, no bindings support that scenario but we get request for it (see realm/realm-java#1129).

Uniqueness is not only useful for primary keys but also for a general case - see realm/realm-java#967.

[simonask]

@kneth Indeed, this is single-column uniqueness.

Multi-column uniqueness can be implemented in a similar way, but we would need a SetMultiple instruction.

[finnschiermer]

I think I like the concepts and the nice explanations, but I'm not going to claim I understand all the implications :-)

[simonask]

@kspangsege, @danielpovlsen, @teotwaki, @rrrlasse I would love your input here as well. :)

[danielpovlsen]

Is this still a proposal or to be considered ready for review?

[simonask]

@danielpovlsen I consider a proposal inherently destined for review. ;-) Sorry if the intention was unclear. Please do review, and please do have opinions about the direction as well! :-)

[teotwaki]

This has the implication that changing the PK of an object to the same value as some other object will actually cause one of the objects to be deleted and the properties of the objects to be merged

I think this is one of the major points, and that discussing this as "primary keys" is dangerous. As far as I can tell, this sort of behaviour is not possible in SQL or even NoSQL stores. @simonask, correct me if I'm wrong, but could this be rephrased as "unique on insert but not on update"?

Are we convinced that what we're providing will fit with user expectations? How many people will expect merges to fail, or merges to be handled in a specific way; and can we properly document that/ensure a specific behaviour? In my opinion, there is a use-case for conflicts to occur, and ask the user for manual intervention.

• Use-case where auto-merge is acceptable: contacts/phone book.
• Use-case where auto-merge is not acceptable: spreadsheet.

[simonask]

It is indeed tricky, because SQL and SQL-like stores provide no such feature directly. Many of them are providing "UPSERT", though, precisely for distributed use cases.

I would argue that this feature is actually in the spirit of "primary keys", in the sense that a primary key is meant to identify an object - logically, if two objects then have the same primary key, they have the same identity, and can therefore be collapsed.

When would you use primary keys in a spreadsheet?

[teotwaki]

Sorry, my brain farted and I'm not entirely sure which point I was trying to get to.

Thinking this over, merge could indeed be the correct default behaviour considering what we're trying to achieve. I guess the main road block I see comes from the TRIGGER concept in RDBMS, which we don't have in Realm, but which would run specific actions upon insertion. In those cases, a merge strategy could have very weird consequences, and is why duplicate PK insertion is blocking in those systems.

[realm-ci]

Coverage: 31%

Please check your coverage here: https://ci.realm.io/job/core_matrix_pr/1413/buildcommand=jenkins-pipeline-coverage,buildtype=debug,encryption=no,slave=fastlinux/Diff_Coverage/

[cmelchior]

Talked a bit with Simon today about this. In my opinion it doesn't matter much if we solve this problem at the ObjectStore level or in Core. For the sake of keeping core as simple as possible (+1 when trying to reason about Sync), I think that solving it at the ObjectStore level would be natural. This means that this proposal would be enough to solve our problems as far as I can see.

Some extra thoughts to consider

• Auto-incremented keys are out (I guess they where never in anyway)
• Synthetic keys like 128 bit GUID's doesn't need to use this operation to function.
• For semantic keys we will have to consider how to resolve conflicts key conflicts. Just merging them with the default conflict resolution could probably work (Hint: Still need support for table-level conflict resolution rules), but will require some extra thinking. E.g. what happens if:

Server: [Andy, Benny, Chris] (name being unique)
Phone A: Renames Andy to Dennis
Phone B: Renames Benny to Dennis

• Adding a setMultipleUnique or similar for supporting composite keys.

[rrrlasse]

SQL has a "merge" keyword, and you can skip conflicting rows by adding a "where not exists" criteria. There also exists a "when not matched then" clause that you can use if you want custom behaviour on conflicts... So SQL is a winner over Realm here, with respect to flexibility.

By the way, do we really need the user to be able to specify/set the unique values? If they were only allowed to be auto generataed, then Realm could use different number generator for each thread that would somehow mathematically be guaranteed never to collide.

[simonask]

@rrrlasse There are two types of primary keys: natural and synthetic. Your suggestion is to disallow natural keys, but we don't think this is what users actually want.

I think it requires a deeper analysis to figure out whether or not we can achieve similar levels of flexibility under this scheme.

[cmelchior]

Disallowing natural keys will not be a good idea IMO. Too many people rely on those (e.g. server generated keys).

Regarding conflict handling, then SQLite e.g. has the following modes: https://www.sqlite.org/lang_conflict.html

[simonask]

@cmelchior Interesting perspective — I hadn't looked into what SQLite actually does. Here is my evaluation of the strategies as they apply to Realm Sync:

• ROLLBACK: Not an option — changes have already been applied on other clients.
• ABORT + FAIL: See above.
• IGNORE: See above. :-)
• REPLACE: This is one possible choice as an outcome. We can also choose the same but opposite solution, where the object causing the PK violation becomes the old object (SQLite would not be able to implement this, as far as I can tell).

[realm-ci]

Coverage: 75%

Please check your coverage here: https://ci.realm.io/job/core_matrix_pr/1429/buildcommand=jenkins-pipeline-coverage,buildtype=debug,encryption=no,slave=fastlinux/Diff_Coverage/

[realm-ci]

Coverage: 78%

Please check your coverage here: https://ci.realm.io/job/core_matrix_pr/1430/buildcommand=jenkins-pipeline-coverage,buildtype=debug,encryption=no,slave=fastlinux/Diff_Coverage/

[realm-ci]

Coverage: 81%

Please check your coverage here: https://ci.realm.io/job/core_matrix_pr/1431/buildcommand=jenkins-pipeline-coverage,buildtype=debug,encryption=no,slave=fastlinux/Diff_Coverage/