neomodel icon indicating copy to clipboard operation
neomodel copied to clipboard

Date properties are stored as string instead of temporal

Open machosec opened this issue 5 years ago • 6 comments

Neomodel's date property types (DateProperty, DateTimeProperty and DateTimeFormatProperty) are inflated and stored as string values in neo4j instead of temporal values (https://neo4j.com/docs/cypher-manual/current/syntax/temporal/). Using string instead of date/time types ruin many filtering/sorting capabilities which I guess are not desired.

machosec avatar Apr 24 '19 07:04 machosec

@machosec I think that what you may be pointing to here is that neomodel does not use a standardised datetime format when storing dates and/or times (?).

With server-side queries, you can always use the functions that cast a date bearing string to a date if you wish to extract specific parts of the date (e.g. in a "group by month" scenario) and at client-side, the date has already been turned into a datetime during filtering.

Is there a more specific example of any failures this might be leading to?

aanastasiou avatar Apr 24 '19 09:04 aanastasiou

Hi @aanastasiou , Neo4j version 3.4 introduced new temporal types and functions to better deal with dates. You can see the example below where the date() function (https://neo4j.com/docs/cypher-manual/current/functions/temporal/date/) returns a temporal type that is treated differently than just a string type. New temporal functions like duration() won't work on a date represented as a string.

image

machosec avatar Apr 24 '19 10:04 machosec

@machosec I understand.

That would be the difference between strptime() and neo4j.v1.types.temporal.Date, etc at this point and of course, the inverse for deflate(). I would have to look more closely the data types that the neo4j-driver returns for dates as it might already be working with straightforward datetimes too.

Unfortunately, the DateTimeFormatProperty might not be as easily handled if it is impossible to represent partial dates.

Feel free to submit a PR if you can handle this.

aanastasiou avatar Apr 25 '19 13:04 aanastasiou

The neotime module used by neo4j-driver support the new temporal types as described here: https://neo4j.com/docs/api/python-driver/current/types/temporal.html I can try to change the inflate and deflate logic of DateProperty and DateTimeProperty to properly store temporal types in neo4j without breaking the existing implementation

machosec avatar Apr 28 '19 07:04 machosec

@machosec That's fine, but if you think you are going to be applying what feels like bigger changes, can you please have a look at this ?.

If you are making big changes, it might be good to see what else could be taken care of at the same time, since you are getting into this "trouble". Note, I am not telling you to do more than providing this feature, I am just telling you that there are a few things open already. If the change you do could also work for something else, that would be great, let's take that into account as early as possible.

All the best

aanastasiou avatar Apr 30 '19 08:04 aanastasiou

Any update on this?

Fedorov113 avatar Jan 28 '21 17:01 Fedorov113