-
-
Notifications
You must be signed in to change notification settings - Fork 480
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
Ensure that latest() is idempotent. #862
Conversation
Codecov Report
@@ Coverage Diff @@
## master #862 +/- ##
=======================================
Coverage 97.67% 97.68%
=======================================
Files 18 19 +1
Lines 990 994 +4
Branches 151 151
=======================================
+ Hits 967 971 +4
Misses 10 10
Partials 13 13
Continue to review full report at Codecov.
|
One job in PR #863 just failed because of this issue. This code is not in that PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code look good.
Description
Two records with the same history_date cannot be retrieved idempotently with latest(). The get_latest_by meta property uses history_date which means if multiple records have the same timestamp, the resulting record returned by the database is not predictable (it would depend on the behavior of the database). If history_date is not indexed the result may be different than if it is indexed, for example (this was observed in #729). The meta property should include sorting by history_id.
Related Issue
This closes #861
Motivation and Context
When latest() is not idempotent, the behavior can change over time (for example adding an index).
How Has This Been Tested?
The unit tests exercise this behavior.
Types of changes
Checklist:
make format
command to format my codeAUTHORS.rst
CHANGES.rst