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

Set<Mixed> should not consider string and binary data equivilent #4860

Closed
sync-by-unito bot opened this issue Aug 20, 2021 · 4 comments
Closed

Set<Mixed> should not consider string and binary data equivilent #4860

sync-by-unito bot opened this issue Aug 20, 2021 · 4 comments
Assignees
Labels

Comments

@sync-by-unito
Copy link

sync-by-unito bot commented Aug 20, 2021

The MongoDB behaviour is to consider a string and a binary different even if they contain the same data, but in a Realm Set they are considered the same. Realm should align to the MongoDB behaviour. In practice this should be a rare case because most strings have a null byte at the end while binaries do not.

Set<Mixed> set;
set.insert(StringData("abc", 3));
set.insert(BinaryData("abc", 3));
REALM_ASSERT(set.size() == 2); // currently fails with size of 1

This change will require a file format bump, but no data migration will be needed.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Aug 21, 2021

➤ Jonathan Reams commented:

The issue here isn't so much that you'd do

Set<Mixed> set;
set.insert(StringData("abc", 3));
set.insert(BinaryData("abc", 3));
REALM_ASSERT(set.size() == 2); // currently fails with size of 1

from a realm SDK, It's that you'd do

db.update({...}, { $addToSet: { set: [ BinData("abc"), "abc" ] } });

from atlas.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Aug 23, 2021

➤ Jørgen Edelbo commented:

We should probably do this when we have to bump the file format version anyway. We will need to re-sort the Set columns.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Aug 23, 2021

➤ Ian Ward commented:

Would this require a breaking API change or anything public facing?

FYI - this feature is still in Beta, so it would be good to make any changes now rather than when we go out of Beta.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Dec 2, 2021

➤ Kiro Morkos commented:

One of our fuzz tests ran into this issue recently, is this going to be picked up soon?

@sync-by-unito sync-by-unito bot closed this as completed Sep 19, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants