search

demetrixbio / FSharp.Data.Npgsql

F# type providers to support statically typed access to input parameters and result set of sql statement in idiomatic F# way. Data modifications via statically typed tables.
Apache License 2.0
127 stars 16 forks source link

DateTime UTC stored incorrectly - 0.1.49-beta #68

Closed sandeepc24 closed 4 years ago

sandeepc24 commented 4 years ago

If datetime value is of type UTC it is converted to Local when storing using AddRow.

daz10000 commented 4 years ago

Timezone issues are my favorite. Can you give a lot more detail on what you are seeing. It’s pretty tricky in Postgres and we’ve been through hell getting this right so want to be really clear about what’s not working. The biggest mistake is usually the incoming datetime value not having right type (eg must be utc or local). Also read carefully what Postgres does with timestamp with tz columns. It’s a bit counterintuitive

Von meinem iPhone gesendet

Am 29.02.2020 um 7:44 PM schrieb Sandeep Chandra notifications@github.com:

 If datetime value is of type UTC it is converted to Local when storing using AddRow.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

sandeepc24 commented 4 years ago

The column in table is timestamp without time zone. Using AddRow, when the value has DateTimeKind of UTC it is converted to local. Even if I make the column timestamp with time zone the value is converted to local.

If I specify DateTime value's DateTimeKind to Unspecified then it is persisted correctly. But I still have to set DateTimeKind after reading from database.

sandeepc24 commented 4 years ago

I think we should follow this https://www.npgsql.org/doc/types/datetime.html

sandeepc24 commented 4 years ago

In Table Detailed Behavior: Sending values to the database the second case is incorrect IMO.

kerams commented 4 years ago

@sandeepc24 These lines are responsible: https://github.com/demetrixbio/FSharp.Data.Npgsql/blob/8976abdfec7bb07f63b261700b7dba429cbcd89a/src/DesignTime/InformationSchema.fs#L204-L213 based on this comment https://github.com/npgsql/npgsql/issues/1076#issuecomment-355400785. It seems it's been put in place to enforce point number 2 in Detailed Behavior: Reading values from the database, but the following comment suggests that it's been fixed, meaning the explicit DataSetDateTime might not be needed anymore. This'll be tricky to fix as the slightest oversight has the potential mess up clients.

The best course of action would be to compile a list of specific test cases for reading/writing DateTimes to the database and expected results.

kerams commented 4 years ago

Looks like I was wrong. @melanore added those lines fairly recently. Maybe he can chime in on this.

sandeepc24 commented 4 years ago

Have a look at this https://github.com/npgsql/npgsql/issues/2881#issuecomment-594392589

sandeepc24 commented 4 years ago

Any update on this issue, I am held up because of this issue.

melanore commented 4 years ago

Conversion to local kind was redundant in case of timestamp column. We are in progress of switching type provider to new SDK template / optimizing design time performance. Fix will be released some time this week.

melanore commented 4 years ago

Issue should be gone in version 0.2.0-beta. https://github.com/demetrixbio/FSharp.Data.Npgsql/pull/70/files#diff-03428eaeb9f35fb62752267fbaa39f96L211

sandeepc24 commented 4 years ago

That fixed the issue for me, thanks a lot.