Issues

Select view

Select search mode

 
50 of

Running Unit tests against SQLite fails on numerous (22) datetime/UTC related tests.

External Issue

Description

Running the tests against SQLite locally on my machine fails 22 tests, while the same commits are passing on TeamCity.

I noticed that the tests have to do with dates and specifically UTC times. For example:

1) Failed : NHibernate.Test.TypesTest.DateTimeTypeFixture.ReadWrite(Utc) Unexpected value. Expected: 2017-10-13 03:23:31.0094937 But was: 2017-10-12 20:23:31.0094937 at NHibernate.Test.TypesTest.AbstractDateTimeTypeFixture.ReadWrite(DateTimeKind kind) in C:\dev\OSS\nhibernate-core\src\NHibernate.Test\TypesTest\AbstractDateTimeTypeFixture.cs:line 176

My computer is in MST or GMT-7:00 timezone.

Attached are all the failures.

Environment

Windows 10 x64, SQLite 1.0.105.1 (included in repository)

Attachments

1
  • 12 Oct 2017, 06:17 AM

Details

Assignee

Frédéric Delaporte

Reporter

Components

Affects versions

Priority

Who's Looking?

Open Who's Looking?
Created October 12, 2017 at 6:17 AM
Updated October 16, 2017 at 10:40 PM
Resolved October 16, 2017 at 10:40 PM

Activity

Show:

Alex Zaytsev October 13, 2017 at 12:12 AM

Could you please continue the conversation at #1362

Frédéric Delaporte October 12, 2017 at 8:04 PM
Edited

it needs to support the edge cases like this for any included types

Only if this can be done in a non hackish, reasonable way. It really looks like an inconsistency in SQLite implementation, that needs to be addressed on its side.

Are there other databases or ADO.Net drivers that do anything like this for non-offset DateTime values?

I have tested the types tests with all the others databases tested with TeamCiry, and with this locally with my French offset, no issues, unlike SQLite.

Since storing DateTime with a UTC marker doesn't really seem like an SQL thing to do, and since all the databases are SQL, then it would probably be best to store all DateTime values as Unspecified, as that was the previous behavior.

I do not believe previous behavior was doing what you write. Previously, the date and time types were not much tested. I think the issue was already there but unnoticed.

Why not forcing the date kind to Unspecified when setting them for avoiding having some databases storing its offset, since the NHibernate DateTimeType type is supposed to be offset agnostic.
It fixes all DateTimeType tests.
But for LocalDateTimeType (which works anyway without this) and UtcDateTimeType (which currently fails), this is more debatable. This later type is for explicitly sending UTC datetime to the database. So sending to it unspecified datetime just for dodging what really looks like a SQLite bug sounds wrong to me.

Then on reading, NHibernate will assume all DateTimes be Unspecified as well (ignore anything passed from the database).

Already the case for UTC and local NHibernate types, just left untouched otherwise. For my test I have changed a method common to both reading and writing for forcing dates to Unspecified when the type is neither UTC nor local, so it was also forcing it back to Unspecified on reads.

I think this issue needs to be fixed on SQLite side. (It is maybe more a System.Data.SQLite issue.)

At the very worst, maybe do the change I have tested (remove the ternary in this line for always forcing kind) then add an AdjustCommand override in SQLite driver for forcing all its datetime parameter to Unspecified even if they were Utc or Local. But that would be a crutch for dodging a SQLite issue, crutch which may cause issues if things change on SQLite side.

Nathan Brown October 12, 2017 at 7:08 PM
Edited

Hmm. That's interesting... I do think that since SQLite is a supported, tested, database for NHibernate, it needs to support the edge cases like this for any included types.

Are there other databases or ADO.Net drivers that do anything like this for non-offset DateTime values?

Since storing DateTime with a UTC marker doesn't really seem like an SQL thing to do, and since all the databases are SQL, then it would probably be best to store all DateTime values as Unspecified, as that was the previous behavior. Then on reading, NHibernate will assume all DateTimes be Unspecified as well (ignore anything passed from the database).

Usually the goal for having a UTC DateTime column in a database is that the developer is acknowledging that timezone offset wasn't stored, but he promises all the values are in the UTC timezone (for reading) and intends to store them in that timezone.

Frédéric Delaporte October 12, 2017 at 11:39 AM
Edited

SQLite default date storage format is ISO8601. There is a gotcha with it: when SQLite receives a local or unspecified kind date, it stores it without any assumption on the time zone. When it receives an UTC one, it stores it with the UTC marker. Why not, sounds rather good.
Trouble arises when reading them back: SQLite converts them all back to local time. So, for dates saved with local or unspecified kind, it keeps them as is. But for UTC dates, it converts them back to local on reading. And worst, it yields them with unspecified kind, leaving us no chance to detect the case and convert them back. (Even with a correct kind, we would not be able to handle converting them back to UTC in all cases, due to ambiguous local time, like 2017-10-29 02:30:00 in France which could be 2017-10-29 01:30:00 UTC or 2017-10-29 00:30:00 UTC, due to time shift backward occurring at 2017-10-29 01:00:00 UTC that day.)

All that terribly looks as an external issue...

The teamcity build has no troubles since its local time zone is UTC.

SQLite has several options about DateTime storage & handling. Its storage format can be changed (some do not handle any time zone indication), and its read kind can be changed (default is Unspecified, but as written above, with an automatic conversion of UTC dates to local beforehand). That further complicates the issue.

DateTimeFormat:

  • ISO8601: distinguishes between UTC dates and others.

  • JulianDay: no differences between UTC dates and others, but only stores milliseconds. Avoids the issue, at the cost of a loss of fractional seconds precision.

  • Ticks: no differences between UTC dates and others, but said as here only for legacy, not recommended by documentation. Avoids the issue.

  • UnixEpoch: no tested but likely like JulianDay but only storing seconds.

  • InvariantCulture: stores the offset with local date. Even more failures due to version check, dates being read back as unspecified.

  • CurrentCulture: not tested.

About Ticks, the documentation writes:

This value is not recommended and is not well supported with LINQ.

Ticks less compatible with 3rd party tools that query the database, and renders the DateTime field unreadable as text without post-processing.

Ticks is mainly present for legacy code support.

So dodging the issue (for tests with a box not using UTC as local time) by using Ticks sounds not good. It actually causes other tests to start failing.

DateTimeKind: it would be quite hackish to play with this one, since currently the NHibernate date types completely ignore read DateTime kind. Most data providers yield it as Unspecified. But indeed it would work to set it to Utc, if we change the read logic to force the datetime back to unspecified with the non local/utc NHibernate date types. (Without a change to read logic, version checks fail due to read value being specified UTC while db value is not.) This DateTimeKind only alters reading, not writing. (At least it does not cause written local datetime to be converted to UTC.) So it eliminates this conversion of UTC to local on reading which causes us issues.
All that sounds too hackish to me for doing such changes.

Another option could be to override SQLite driver for switching to Unspecified all dates parameters, in order to avoid having their offset/utc flag stored when the underlying SQLite date is able of storing it. This is still a hack for compensating SQLite behavior.

Overall I think this is a SQLite issue, NHibernate code should not try to fix it.

So what to do with those tests failing on boxes having a local time other than UTC? It it worth coding some ignore logic for them? Or just let them fail, since it does not hinder TeamCity?

Who's Looking?