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.

Sign up for GitHub

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

Jump to bottom

Update the from_sorter_iter method to return an option #103

Closed
Kerollmops opened this issue May 2, 2021 · 0 comments · Fixed by #106
Closed

Update the from_sorter_iter method to return an option #103

Kerollmops opened this issue May 2, 2021 · 0 comments · Fixed by #106
Labels
breaking This change will require a bump of the minor or major version. bug good first issue Good for newcomers

Comments

@Kerollmops
Copy link
Member

We recently updated the RoaringBitmap::push method to make it panic-free but we forgot about the RoaringBitmap::from_sorted_iter method, it was previously panicking but it is now ignoring unsorted insertions.
@Kerollmops Kerollmops added bug breaking This change will require a bump of the minor or major version. good first issue Good for newcomers labels May 2, 2021
@Kerollmops Kerollmops mentioned this issue May 25, 2021
Merged
@bors bors bot closed this as completed in e0d6477 Oct 21, 2021
not-jan pushed a commit to not-jan/roaring-rs that referenced this issue Aug 31, 2022
@bors @Kerollmops
Merge RoaringBitmap#106
6ee1267 
106: Modify the from_sorted_iter and append methods to return a Result r=Kerollmops a=Kerollmops

This PR fixes RoaringBitmap#103, which means that the `from_sorted_iter` and `append` methods of the `RoaringBit/Treemap` types now return a `Result`. The `Err` value corresponds to the number of values that we appended to the set until we found an erroneous value, it eases debugging.

These two methods were containing bugs in the sense that they were silently discarding invalid values instead of panicking, this bug was introduced by RoaringBitmap#99 which introduced a non-panicking push method.

A possible improvement could be to introduce a new error type, something like a `NonSortedIntegers` struct with a `stopped_at`/`valid_until` method that returns the `u64` we currently return as the `Err`.

`@MarinPostma` could you please take a look at this PR? If you got the time?

Co-authored-by: Kerollmops <clement@meilisearch.com>
Co-authored-by: Clément Renault <renault.cle@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
breaking This change will require a bump of the minor or major version. bug good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Modify the from_sorted_iter and append methods to return a Result RoaringBitmap/roaring-rs
1 participant
@Kerollmops