neomodel icon indicating copy to clipboard operation
neomodel copied to clipboard

DateTime datatype does not seem to reflect neo4j datatype

Open mikemiles-dev opened this issue 4 years ago • 7 comments

When using the DateTime Property datatype on a StructuredNode it appears when the data ends up in Neo4J the datatype is actually a double, which then I cannot use any Neo4j datetime queries with it without casting.

mikemiles-dev avatar Jul 29 '20 17:07 mikemiles-dev

Using Neo4j 4.0+

mikemiles-dev avatar Jul 29 '20 17:07 mikemiles-dev

neomodel does not officially support Neo4j>=3.4. Especially Neo4j>=4 is major version change so it is difficult to support. See #485

elferia avatar Jul 29 '20 18:07 elferia

@justmike2000 can you please update to the latest version and let us know if this persists?

aanastasiou avatar Sep 19 '20 13:09 aanastasiou

@justmike2000 can you please update to the latest version and let us know if this persists?

@aanastasiou Looking at the code, the issue persists. I think the question here is about neomodel converting datetimes to floats (which Neo4j call Doubles), instead of using the recently polished native Neo4j datetimes. The relevant neomodel code: return float((value - epoch_date).total_seconds()). Previously, Neo4j didn't have very good datetime support (if any), so I assume that's why it isn't implemented, but that has since changed (in fact Neo4j native datetimes are now more precise than regular Python datetimes, due to nanosecond precision).

Writing a fix for this is pretty easy (having recently monkey-patched my local instance of neomodel), but requires that we make a decision on how to deal with instances coming from the old values. For instance, in the case of this issue, datetimes will be stored as Python floats in Neo4j, but inflated to Python datetime via neomodel. If we change how we store things in neo4j by turning it into Neo4j datetimes, we leave nodes that have not been processed by neomodel since the change, in an inconsistent state (some nodes will have the property as float, some as datetime). So either we convert values on the fly, meaning users need to re-save every single node using a DatetimeProperty (we could potentially add a management command to help in this regard), or we add a flag that allows users to control how the property should be stored on the database end.

We also have to keep in mind that the Python datetime object is less precise than the Neo4j datetime object. This could likely just be "fixed" with a documentation update.

JVemmer avatar Oct 22 '20 11:10 JVemmer

@JVemmer would you be willing to share your monkeypatch ;)?

yeus avatar Nov 22 '20 22:11 yeus

alright... @JVemmer nevermind it was really quick. For those who are interested:

here is the code for a custom datetime class supporting the neo4j datetime format:

from neomodel import Property
import neomodel.properties
from datetime import datetime


class DateTimeFormatNeo4j(Property):
    """
    Store a datetime by native neo4j format
    """
    form_field_class = 'DateTimeFormatFieldNeo4j'

    def __init__(self, default_now=False, **kwargs):
        if default_now:
            if 'default' in kwargs:
                raise ValueError('too many defaults')
            kwargs['default'] = lambda: datetime.now()

        self.format = format
        super(DateTimeFormatNeo4j, self).__init__(**kwargs)

    @neomodel.properties.validator
    def inflate(self, value):
        return value.to_native()

    @neomodel.properties.validator
    def deflate(self, value):
        if not isinstance(value, datetime):
            raise ValueError('datetime object expected, got {0}.'.format(type(value)))
        return neo4j.time.DateTime.from_native(value)

Inserting such a class into the library wouldn't be such a big proble wouldn't it? it would also not break backwards compatibility. Whoever wants to "upgrade" their database ould just do that using a cypher similar to this:

MATCH (n)
SET n.my_datetime_property=datetime()

yeus avatar Nov 22 '20 23:11 yeus

I created this little pull-request here:

https://github.com/neo4j-contrib/neomodel/pull/530

i order to discuss this

yeus avatar Nov 22 '20 23:11 yeus