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

Fix Time comparisons with _matches? to use millisecond precision. #5094

Merged
merged 6 commits into from
Nov 17, 2021
Merged

Fix Time comparisons with _matches? to use millisecond precision. #5094

merged 6 commits into from
Nov 17, 2021

Conversation

jonhyman
Copy link
Contributor

Updates _matches? to compare Time objects using millisecond precision like what MongoDB does. We have a selector that returns a result that doesn't meet _matches? because of nanosecond rounding. This pull does something similar to what the bson-ruby gem does at https://github.com/mongodb/bson-ruby/blob/master/lib/bson/time.rb#L57 and compares Time objects using millisecond precision.

Per https://docs.mongodb.com/ruby-driver/current/tutorials/bson-v4/#time-instances,

Times in BSON (and MongoDB) can only have millisecond precision. When Ruby Time instances are serialized to BSON or Extended JSON, the times are floored to the nearest millisecond.
...
Because of this flooring, applications are strongly recommended to perform all time calculations using integer math, as inexactness of floating point calculations may produce unexpected results.

p-mongo
p-mongo reviewed Oct 28, 2021
View reviewed changes
Copy link
Contributor

@p-mongo p-mongo left a comment

Choose a reason for hiding this comment

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

@jonhyman Thank you for the PR! It looks good except I think the array comparisons should be subject to the same time comparison logic as the scalars, do you agree?

lib/mongoid/matcher/eq_impl.rb Show resolved Hide resolved
@jasonpenny jasonpenny mentioned this pull request Oct 28, 2021
Merged
@p-mongo
Copy link
Contributor

p-mongo commented Oct 29, 2021

@jonhyman can you also please merge master into this PR, it's based on a very old master.

@jonhyman
Copy link
Contributor Author

Master is merged in.

@comandeo comandeo merged commit b7acf87 into mongodb:master Nov 17, 2021
comandeo pushed a commit to comandeo/mongoid that referenced this pull request Nov 17, 2021
@jonhyman @jasonpenny @comandeo-mongo
Fix Time comparisons with _matches? to use millisecond precision. (mo…
e1828b3 
…ngodb#5094)

Co-authored-by: Jason Penny <[email protected]>
Co-authored-by: Dmitry Rybakov <[email protected]>
@comandeo
Copy link
Contributor

https://jira.mongodb.org/browse/MONGOID-5195

@comandeo
Copy link
Contributor

@jonhyman @jasonpenny Thank you for the contribution!

@jasonpenny jasonpenny mentioned this pull request Nov 17, 2021
Merged
@jasonpenny jasonpenny deleted the millis-precision branch December 16, 2021 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonpenny jasonpenny jasonpenny left review comments

@p-mongo p-mongo p-mongo left review comments

@comandeo comandeo comandeo approved these changes

Assignees
No one assigned
Labels
dmitry-todo
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jonhyman @p-mongo @comandeo @jasonpenny @comandeo-mongo