Skip to content

Microsecond bug in neo4j.time.DateTime.to_native() #525

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

Closed
havardthom opened this issue Apr 6, 2021 · 3 comments
Closed

Microsecond bug in neo4j.time.DateTime.to_native() #525

havardthom opened this issue Apr 6, 2021 · 3 comments

Comments

@havardthom
Copy link

havardthom commented Apr 6, 2021

Converting a neo4j.time.DateTime to the built-in datetime.datetime using your to_native method will sometimes give wrong microseconds. I suspect this is a rounding error related somehow to this line:

ms = int(nano_mul(ns, 1000000))

Driver version: Python driver 4.2.1
Operating System: CentOS Linux 7

Steps to reproduce

from datetime import datetime
from neo4j.time import DateTime as Neo4jDateTime

timestamps = [
    "2021-04-06T00:00:00.400009+00:00",
    "2021-04-06T00:00:00.400008+00:00",
    "2021-04-06T00:00:00.400007+00:00",
    "2021-04-06T00:00:00.400006+00:00",
    "2021-04-06T00:00:00.400005+00:00",
    "2021-04-06T00:00:00.400004+00:00",
    "2021-04-06T00:00:00.400003+00:00",
    "2021-04-06T00:00:00.400002+00:00",
    "2021-04-06T00:00:00.400001+00:00",
    "2021-04-06T00:00:00.600009+00:00",
    "2021-04-06T00:00:00.600008+00:00",
    "2021-04-06T00:00:00.600007+00:00",
    "2021-04-06T00:00:00.600006+00:00",
    "2021-04-06T00:00:00.600005+00:00",
    "2021-04-06T00:00:00.600004+00:00",
    "2021-04-06T00:00:00.600003+00:00",
    "2021-04-06T00:00:00.600002+00:00",
    "2021-04-06T00:00:00.600001+00:00",
    "2021-04-06T00:00:00.500009+00:00",
    "2021-04-06T00:00:00.500008+00:00",
    "2021-04-06T00:00:00.500007+00:00",
    "2021-04-06T00:00:00.500006+00:00",  # Fails by 1 microsecond
    "2021-04-06T00:00:00.500005+00:00",
    "2021-04-06T00:00:00.500004+00:00",
    "2021-04-06T00:00:00.500003+00:00",
    "2021-04-06T00:00:00.500002+00:00", # Fails by 1 microsecond
    "2021-04-06T00:00:00.500001+00:00",
]

for timestamp in timestamps:
    neo4j_datetime = Neo4jDateTime.from_iso_format(timestamp)
    native_from_neo4j = neo4j_datetime.to_native()
    native_from_datetime = datetime.fromisoformat(timestamp)

    print("neo4j_datetime: ", neo4j_datetime)
    print("native_from_neo4j: ", native_from_neo4j)
    print("native_from_datetime: ", native_from_datetime)
    print()

    assert neo4j_datetime == native_from_datetime
    assert native_from_neo4j == native_from_datetime

Result:

...

neo4j_datetime: 2021-04-06T00:00:00.500007000+00:00
native_from_neo4j: 2021-04-06T00:00:00.500007+00:00
native_from_datetime: 2021-04-06T00:00:00.500007+00:00

neo4j_datetime: 2021-04-06T00:00:00.500006000+00:00
native_from_neo4j: 2021-04-06T00:00:00.500005+00:00
native_from_datetime: 2021-04-06T00:00:00.500006+00:00

assert native_from_neo4j == native_from_datetime
AssertionError

Expected behavior

native_from_neo4j should always be the same as native_from_datetime in my example.

Actual behavior

native_from_neo4j is sometimes off by 1 microsecond compared to native_from_datetime

@havardthom
Copy link
Author

Did some more digging and it seems the problem is from float multiplication at this line

nx = int(1000000000 * x)

Where
x = 0.500006 ->
1000000000 * x = 500005999.99999994 ->
int(500005999.99999994) = 500005999

A simple solution is accepting the floating point arithmetic error and just round to closest number before casting to int.
A more complex solution could be separating seconds and milliseconds calculations so they could be dealt with as ints instead of floats to avoid floating point arithmetic

@robsdedude
Copy link
Member

Thanks for the investigation. I also had a look at it an will most likely take the second approach, storing integer nanoseconds instead of float sub-seconds.

robsdedude added a commit to robsdedude/neo4j-python-driver that referenced this issue Jun 14, 2021
robsdedude added a commit to robsdedude/neo4j-python-driver that referenced this issue Jun 14, 2021
robsdedude added a commit to robsdedude/neo4j-python-driver that referenced this issue Sep 20, 2021
@robsdedude
Copy link
Member

The fix will be available in 4.4 it required slightly altering the API so I can't publish it in a patch release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants