Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RealmObject should overload GetHashCode #1650

Closed
nielsenko opened this issue Nov 29, 2017 · 12 comments · Fixed by #2296
Closed

RealmObject should overload GetHashCode #1650

nielsenko opened this issue Nov 29, 2017 · 12 comments · Fixed by #2296

Comments

@nielsenko
Copy link
Contributor

nielsenko commented Nov 29, 2017

Goals

Feature request: Have RealmObject overload GetHashCode to match the overload of Equals. I
I think it i "bad" c#-style not to do so - it even gives a usage warning: CA2218: Override GetHashCode on overriding Equals on compilation (https://docs.microsoft.com/en-us/visualstudio/code-quality/ca2218-override-gethashcode-on-overriding-equals).

For extra bonus, have GetHashCode work even if IsValid == false if possible. This will allow us to cleanup any mapped objects we may have stored in hash tables.

@bmunkholm
Copy link
Contributor

Hi @nielsenko
Seems like a good idea. Anyone up for a PR to help out for eternal fame and glory ;-)? That would be lovely!

@janniklind
Copy link

janniklind commented Nov 29, 2017

Seems others are struggling with the same thing. #1600, maybe we could get a copy of the object, an unmanaged copy.

The row-index in the ObjectHandle, can that be used as HashCode for the RealmObject? After deletion will the row-index be reused or is it always incremented when new objects get an index.
If it is never reused, I think we could use that. If it is reused, I think the copy solution is fine, then the user has an accessible object from whom he can calculate the HashCode from.

@nirinchev
Copy link
Member

@janniklind you can't get a copy of the object, because when you factor relationships and backlinks you may end up pulling the entire database in memory :) That being said, tools like AutoMapper allow you to specify a mapping strategy and copy the relevant fields of a managed object to memory.

@nielsenko the GetHashCode method should be fairly easy to implement but it's very unlikely that it can be stable between managed and unmanaged objects, e.g. if I have

var foo = new Foo();
var unmanagedHash = foo.GetHashCode();

realm.Write(() => realm.Add(foo));

var managedHash = foo.GetHashCode();

// managedHash != unmanagedHash

And as Brian said, we'd be very happy to review a PR that adds this.

@nielsenko
Copy link
Contributor Author

Well, since you ask so nicely :-)

I have been looking a bit at the source. I get two fody related errors when trying to build master with vs4mac, but I'll try anyway.

My idea is to use delegate from RealmObject.GetHashCode() to ObjectHandle.GetHashCode() and from there to handle.GetHashCode() (hence it will only work when IsValid == true and hence handle != IntPtr.Zero).

As I recall IntPtr.GetHashCode() gives us a sensible and stable value, as long as you don't compact the db on the fly, the handle should be stable - am I right?

@fealebenpae
Copy link
Member

Note that it’s entirely possible to get two or more distinct handles pointing to the same row in the database.

@nielsenko
Copy link
Contributor Author

Yes, but the underlying IntPtr you get from:

public static extern IntPtr get_row_index(ObjectHandle objectHandle, out NativeException ex);

stays the same, as long as the object is alive, and the realm db is not compacted, which never happens on the fly, or?

@nielsenko nielsenko mentioned this issue Nov 29, 2017
3 tasks
@fealebenpae
Copy link
Member

The row index is merely a sequential integer. We use IntPtr because the size of the integer is platform-specific, but in essence the row index is literally the zero-based index of the row in its table. You could use that to infer a hash code, but you will need to take into account the version of the realm as well because row indexes are not guaranteed to be stable across transactions.

The best way to overcome the ambiguity of the row index is to also use the pointer to the table it belongs to via RealmObject.Metadata.Table. This will produce false negatives when comparing the same object (with the same row index) from two different instances of the same realm (across threads, for example), but at least it won't produce false positives when comparing row indexes from different tables/realms.

Once Core switches away from row indexes and assigns every row a unique id that will be stable across versions instead we should be able to implement GetHashCode() in terms of that unique id. I believe that work is on-going, but in the mean time hashing a row index and its table handle should be good enough, with a couple of caveats that object ids will fix.

@janniklind
Copy link

@fealebenpae is there any ETA for the unique id instead of indexes?

@fealebenpae
Copy link
Member

@janniklind early next year, I guess. It's a major change.

@nielsenko
Copy link
Contributor Author

@fealebenpae You say that row indexes are not guaranteed to be stable across transactions, and, if I understand you correctly, including RealmObject.Metadata.Table won't help with that. It will however help us avoid collisions between realm objects of different type (from different tables).

To have a stable hash code across transactions I need to take into account the version of the realm. I must admit, that it is not apparent to me how to to build a stable hash from a changing row index and realm version.

For the use cases I see, I need the hash to be stable while the process is running. It need not be stable across processes, so I feel it should be possible to calculate a hash without a unique row id, if just there is something that is stable per object while the process runs.

Anyway, I have another idea..

To work around the missing overload of GetHashCode() I have typically implemented it myself, by adding a primary key to the object and using that as the stable base, and I assume Fody could do something similar during weaving. Would that be better? At least until you get unique row ids in core?

@fealebenpae
Copy link
Member

Primary keys would be a lot better, I think, yes.

@nirinchev
Copy link
Member

I can look into adding the weaver logic later this month if no one wants to beat me to it.

@nirinchev nirinchev self-assigned this Mar 16, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants