Zaid-Ajaj / LiteDB.FSharp

Advanced F# Support for LiteDB, an embedded NoSql database for .NET with type-safe query expression through F# quotations
MIT License
180 stars 22 forks source link

DateTime read back from db is incorrect #46

Closed sandeepc24 closed 3 years ago

sandeepc24 commented 4 years ago

This is not an LiteDB.FSharp but I was wondering if it can be fixed in FSharpBsonMapper?

Zaid-Ajaj commented 4 years ago

Hello @sandeepc24, thanks a lot for filing the issue! If there is something wrong with a DateTime roundtrip, I would like to fix it ASAP, perhaps you have a small example where the library isn't working according to your expectations? Please share it here so I can test against it.

humhei commented 4 years ago

Maybe related to #43 My workaround is using timestap int64 instead of DateTime.

This is because even in Lite.DB repository, There is also a default culture setting to decide whether to deserialize datetime using local culture or global culture..(if i memory currently, it defalult using local culture)

While in litedb.fsharp, the behavior is a litter different from liteDb. So the error was occured

sandeepc24 commented 4 years ago

After more investigation it looks like the datetime is stored as UTC but not converted back to local time. This caused the confusion.

I think it is incorrect for LiteDB to convert DateTime to UTC, it is a db and should just store the value as given to it without converting it to UTC.

To fix this I could add a function to ResolveMember and convert the value to LocalTime.

sandeepc24 commented 4 years ago

It looks like the Deserialize method is not working, have a look at the following code

let dateTimeResolver = 
    Action<Type, Reflection.MemberInfo, MemberMapper>(
        fun typ memberInfo mbr ->
            if mbr.DataType = typeof<DateTime> then
                mbr.Deserialize <- fun v m -> 
                    v.AsDateTime.ToLocalTime() :> obj

let private mapper = FSharpBsonMapper()
mapper.ResolveMember <- dateTimeResolver
let getDb() =
    new LiteDatabase(sprintf "Filename=%A;Connection=shared" dbPath, mapper)

Deserialize function is never called.

sandeepc24 commented 4 years ago

A workaround is to store dates as UTC (DateTime.Kind = Utc) then there is no conversion and the values read back are correct.

Zaid-Ajaj commented 4 years ago

Hello @sandeepc24, it is indeed correct that DateTime values are stored as UTC times because we can't make assumptions about the local time of the application you are using. For example, if you are using LiteDB in a website, then a local time has different meaning for different users across different time zones, so the default is to use UTC and convert to Local Time as needed.

However, since the chance is high that LiteDB is only using Local Time for desktop or mobile apps, it would make sense to make it configurable from the FSharpBsonMapper to allow using Local Time without UTC convertion:

let mapper = FSharpBsonMapper(convertDateTimeUtc = false)

What do you think about this? cc @humhei

humhei commented 4 years ago

good idea 👍 BTW, we should apply convertDateTimeUtc option both in serialization and deserialization

magnushammar commented 4 years ago

DateTime will always be context sensitive and should imo not be touched (converted) by the storage layer. DateTimeOffset is needed to properly record Local & UTC time.

Zaid-Ajaj commented 3 years ago

As of v2.16 LiteDB.FSharp will not change the value back to UTC when persisting DateTime instances