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

RUST-2017 Add method to construct an ObjectId from its parts #492

Merged
merged 4 commits into from
Aug 27, 2024
Merged

RUST-2017 Add method to construct an ObjectId from its parts #492

merged 4 commits into from
Aug 27, 2024

Conversation

tyilo
Copy link
Contributor

@tyilo tyilo commented Aug 19, 2024

No description provided.

@abr-egn
Copy link
Contributor

abr-egn commented Aug 20, 2024

Can I ask about the use case for this? Synthesizing ObjectIds seems likely to run into unexpected collisions or other subtle issues.

@abr-egn abr-egn self-assigned this Aug 20, 2024
@tyilo
Copy link
Contributor Author

tyilo commented Aug 21, 2024 •
edited
Loading

Can I ask about the use case for this?

The use case is that I want to find documents in a specific time interval. The collection doesn't have any indexes, however the auto-created _id can be used for this.

Synthesizing ObjectIds seems likely to run into unexpected collisions or other subtle issues.

This is already possible using ObjectId::from_bytes

@abr-egn abr-egn self-requested a review August 22, 2024 14:31
abr-egn
abr-egn approved these changes Aug 22, 2024
View reviewed changes
Copy link
Contributor

@abr-egn abr-egn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@abr-egn abr-egn requested a review from isabelatkinson August 22, 2024 18:29
isabelatkinson
isabelatkinson reviewed Aug 26, 2024
View reviewed changes
Copy link
Contributor

@isabelatkinson isabelatkinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes look good, can we add a simple test for this in src/tests/modules/oid.rs?

src/oid.rs Outdated Show resolved Hide resolved
@tyilo
Copy link
Contributor Author

tyilo commented Aug 27, 2024

changes look good, can we add a simple test for this in src/tests/modules/oid.rs?

Done

isabelatkinson
isabelatkinson approved these changes Aug 27, 2024
View reviewed changes
Copy link
Contributor

@isabelatkinson isabelatkinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks for your contribution!

@isabelatkinson isabelatkinson changed the title Allow constructing an ObjectId from its parts RUST-2017 Add method to construct an ObjectId from its parts Aug 27, 2024
@isabelatkinson isabelatkinson merged commit 20c56f0 into mongodb:main Aug 27, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@isabelatkinson isabelatkinson isabelatkinson approved these changes

@abr-egn abr-egn abr-egn approved these changes

Assignees

@abr-egn abr-egn

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tyilo @abr-egn @isabelatkinson