Fix Time value precision calculation #667
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
From MS SQL docs, the datetime type has a millisecond precision of 1/300th of a second, or more specifically:
The current implementation tries to round values passed as
datetimeto this standard, however the implementation is giving wrong values that do not end in either 0, 3 or 7. For example, if we pass a date ending in, say,.8666667seconds, it will incorrectly round to.865instead of.867, which would be the correct rounding for SQL Server, and also the correct rounding anywhere.What I did was to eliminate a couple of inversions that were being done without any good reason (in fact they only made the result stray further from the right value), and increase the precision of the divider being applied (to make the overall precision 1/300th of a second).
Now milliseconds ending in
9are being rounded to10, or more precisely, to0. This means that a date of2017-11-25 10:05:38.999499should be rounded to2017-11-25 10:05:39.000, but was instead being rounded to2017-11-25 10:05:38.000. There's actually an open issue about this. The second part of this PR addresses this. We now detect if there's a "carry second", and add it to the final date, after manually changing the microseconds to the rounded value.Overall, because of the removed inversion logic, this conversion does not add any extra execution time, and this can demonstrated with a benchmark (which I will soon add here).