Fix invalid cast exception for DateTimeOffset type

[amul047]

Fixes #2820

[amul047]

I can't add a linked issue, but this PR relates to #2820

[bahusoid]

It seems we have tests to cover DateTimeOffset. See DateTimeOffsetQueryFixture. Why tests work without this changes? Can you add a failing test case there.

[bahusoid]

Ah... It is specific to column type timestamptz (which is not what is created by NHIbernate by default)

[amul047]

Ah... It is specific to column type timestamptz (which is not what is created by NHIbernate by default)

Are you saying that if we had RegisterColumnType(DbType.DateTimeOffset, "timestamptz"); over at

nhibernate-core/src/NHibernate/Dialect/PostgreSQLDialect.cs

Line 139 in 47992e7

protected virtual void RegisterDateTimeTypeMappings()

, then we won't have this issue at all?

[amul047]

Ah... It is specific to column type timestamptz (which is not what is created by NHIbernate by default)

Are you saying that if we had RegisterColumnType(DbType.DateTimeOffset, "timestamptz"); over at

nhibernate-core/src/NHibernate/Dialect/PostgreSQLDialect.cs

Line 139 in 47992e7

protected virtual void RegisterDateTimeTypeMappings()

, then we won't have this issue at all?

It looks like we do this for another dialect

nhibernate-core/src/NHibernate/Dialect/SybaseSQLAnywhere12Dialect.cs

Line 74 in 47992e7

RegisterColumnType(DbType.DateTimeOffset, "TIMESTAMP WITH TIME ZONE");

timestamptz is just short for TIMESTAMP WITH TIME ZONE in PostgresQL

[amul047]

Also, it appears that DateTimeOffsetFixture tests are being ignored for PostgresQL - https://teamcity.jetbrains.com/viewLog.html?buildId=3511808&buildTypeId=bt875&tab=testsInfo .

[hazzik]

Need tests.

[gliljas]

AFAIK, no RDBMS other than SQL Server can actually handle a DateTimeOffset with any credibility. Some, like Postgres, can certainly store it, but the value is converted to UTC and the offset is thrown away.

[fredericDelaporte]

See my comments in #2820. Gunnar is right. And I think we should not fake a DateTimeOffset support when it is not actually fully supported. So this PR would have to be rejected.

From Npgsql documentation:

A common mistake is for users to think that the PostgreSQL timestamp with timezone type stores the timezone in the database. This is not the case: only the timestamp is stored. There is no single PostgreSQL type that stores both a date/time and a timezone, similar to .NET DateTimeOffset.

[bahusoid]

Original fix contains only 6f948f6. And I see no harm in this change. I agree we shouldn't add timestampz registrations for DateTimeOffset for PostreSQL if it's not supported properly. But why should we restrict custom mapping? It's user responsibility for data correctness in this case. I think original fix is good move - requesting exact type is better than typing after retrieving object. Dialect might handle it more effectively.

[gliljas]

Yes, that change makes sense. Perhaps even on a broader scale, not only for DateTimeOffset.

[bahusoid]

And... It fails the same on PostgreSQL (Npgsql Version="4.1.9"):

System.InvalidCastException : Unable to cast object of type 'System.DateTime' to type 'System.DateTimeOffset'.
[00:08:09]    at NHibernate.Loader.Loader.DoList(ISessionImplementor session, QueryParameters queryParameters, IResultTransformer forcedResultTransformer, QueryCacheResultBuilder queryCacheResultBuilder) in C:\projects\nhibernate-core\src\NHibernate\Loader\Loader.cs:line 1981
[00:08:09]    at NHibernate.Loader.Loader.ListIgnoreQueryCache(ISessionImplementor session, QueryParameters queryParameters) in C:\projects\nhibernate-core\src\NHibernate\Loader\Loader.cs:line 1837
[00:08:09]    at NHibernate.Hql.Ast.ANTLR.QueryTranslatorImpl.List(ISessionImplementor session, QueryParameters queryParameters) in C:\projects\nhibernate-core\src\NHibernate\Hql\Ast\ANTLR\QueryTranslatorImpl.cs:line 119
[00:08:09]    at NHibernate.Engine.Query.HQLQueryPlan.PerformList(QueryParameters queryParameters, ISessionImplementor session, IList results) in C:\projects\nhibernate-core\src\NHibernate\Engine\Query\HQLQueryPlan.cs:line 116
[00:08:09]    at NHibernate.Impl.SessionImpl.List(IQueryExpression queryExpression, QueryParameters queryParameters, IList results, Object filterConnection) in C:\projects\nhibernate-core\src\NHibernate\Impl\SessionImpl.cs:line 559
[00:08:09]    at NHibernate.Impl.SessionImpl.List(IQueryExpression queryExpression, QueryParameters queryParameters, IList results) in C:\projects\nhibernate-core\src\NHibernate\Impl\SessionImpl.cs:line 524
[00:08:09]    at NHibernate.Impl.AbstractSessionImpl.List[T](IQueryExpression query, QueryParameters parameters) in C:\projects\nhibernate-core\src\NHibernate\Impl\AbstractSessionImpl.cs:line 183
[00:08:09]    at NHibernate.Impl.AbstractQueryImpl2.List[T]() in C:\projects\nhibernate-core\src\NHibernate\Impl\AbstractQueryImpl2.cs:line 108
[00:08:09]    at NHibernate.Linq.DefaultQueryProvider.ExecuteList[TResult](Expression expression) in C:\projects\nhibernate-core\src\NHibernate\Linq\DefaultQueryProvider.cs:line 114
[00:08:09]    at NHibernate.Linq.NhQueryable`1.System.Collections.Generic.IEnumerable<T>.GetEnumerator() in C:\projects\nhibernate-core\src\NHibernate\Linq\NhQueryable.cs:line 65
[00:08:09]    at System.Collections.Generic.List`1.AddEnumerable(IEnumerable`1 enumerable)
[00:08:09]    at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
[00:08:09]    at NHibernate.Test.NHSpecificTest.GH2820.PostgresTimestampzFixture.CanRetrieveEntityWithTimeStampColumn() in C:\projects\nhibernate-core\src\NHibernate.Test\NHSpecificTest\GH2820\FixtureByCode.cs:line 59
[00:08:09] --FormatException
[00:08:09]    at NHibernate.Type.DateTimeOffsetType.Get(DbDataReader rs, Int32 index, ISessionImplementor session) in C:\projects\nhibernate-core\src\NHibernate\Type\DateTimeOffSetType.cs:line 72
[00:08:09]    at NHibernate.Type.NullableType.NullSafeGet(DbDataReader rs, String name, ISessionImplementor session) in C:\projects\nhibernate-core\src\NHibernate\Type\NullableType.cs:line 235
[00:08:09]    at NHibernate.Persister.Entity.AbstractEntityPersister.Hydrate(DbDataReader rs, Object id, Object obj, String[][] suffixedPropertyColumns, ISet`1 fetchedLazyProperties, Boolean allProperties, Int32[] indexes, ISessionImplementor session) in C:\projects\nhibernate-core\src\NHibernate\Persister\Entity\AbstractEntityPersister.cs:line 2854
[00:08:09]    at NHibernate.Persister.Entity.LoadableExtensions.Hydrate(ILoadable loadable, DbDataReader rs, Object id, Object obj, String[][] suffixedPropertyColumns, ISet`1 fetchedLazyProperties, Boolean allProperties, ISessionImplementor session) in C:\projects\nhibernate-core\src\NHibernate\Persister\Entity\ILoadable.cs:line 101
[00:08:09]    at NHibernate.Loader.Loader.LoadFromResultSet(DbDataReader rs, Int32 i, Object obj, ILoadable persister, EntityKey key, LockMode lockMode, ILoadable rootPersister, ISessionImplementor session) in C:\projects\nhibernate-core\src\NHibernate\Loader\Loader.cs:line 1301
[00:08:09]    at NHibernate.Loader.Loader.InstanceNotYetLoaded(DbDataReader dr, Int32 i, ILoadable persister, EntityKey key, LockMode lockMode, EntityKey optionalObjectKey, Object optionalObject, IList hydratedObjects, ISessionImplementor session) in C:\projects\nhibernate-core\src\NHibernate\Loader\Loader.cs:line 1164
[00:08:09]    at NHibernate.Loader.Loader.GetRow(DbDataReader rs, ILoadable[] persisters, EntityKey[] keys, Object optionalObject, EntityKey optionalObjectKey, LockMode[] lockModes, IList hydratedObjects, ISessionImplementor session, Boolean mustLoadMissingEntity, Action`2 cacheBatchingHandler) in C:\projects\nhibernate-core\src\NHibernate\Loader\Loader.cs:line 1041
[00:08:09]    at NHibernate.Loader.Loader.GetRowFromResultSet(DbDataReader resultSet, ISessionImplementor session, QueryParameters queryParameters, LockMode[] lockModeArray, EntityKey optionalObjectKey, IList hydratedObjects, EntityKey[] keys, Boolean returnProxies, IResultTransformer forcedResultTransformer, QueryCacheResultBuilder queryCacheResultBuilder, Action`2 cacheBatchingHandler) in C:\projects\nhibernate-core\src\NHibernate\Loader\Loader.cs:line 398
[00:08:09]    at NHibernate.Loader.Loader.DoQuery(ISessionImplementor session, QueryParameters queryParameters, Boolean returnProxies, IResultTransformer forcedResultTransformer, QueryCacheResultBuilder queryCacheResultBuilder) in C:\projects\nhibernate-core\src\NHibernate\Loader\Loader.cs:line 558
[00:08:09]    at NHibernate.Loader.Loader.DoQueryAndInitializeNonLazyCollections(ISessionImplementor session, QueryParameters queryParameters, Boolean returnProxies, IResultTransformer forcedResultTransformer, QueryCacheResultBuilder queryCacheResultBuilder) in C:\projects\nhibernate-core\src\NHibernate\Loader\Loader.cs:line 303
[00:08:09]    at NHibernate.Loader.Loader.DoList(ISessionImplementor session, QueryParameters queryParameters, IResultTransformer forcedResultTransformer, QueryCacheResultBuilder queryCacheResultBuilder) in C:\projects\nhibernate-core\src\NHibernate\Loader\Loader.cs:line 1972
[00:08:09] --InvalidCastException
[00:08:09]    at System.Data.Common.DbDataReader.GetFieldValue[T](Int32 ordinal)
[00:08:09]    at NHibernate.Type.DateTimeOffsetType.Get(DbDataReader rs, Int32 index, ISessionImplementor session) in C:\projects\nhibernate-core\src\NHibernate\Type\DateTimeOffSetType.cs:line 68

[bahusoid]

Maybe because of our NHybridDataReader...

[bahusoid]

So it seems fix requires specific type checks like:

var value = rs.GetValue(index);
if (value is DateTime dateTime)
	return (DateTimeOffset) dateTime;

So yeah maybe it's not worth it as you guys already said... @amul047 If you really need broken DateTimeOffset you can create custom type with this logic and register it globally with TypeFactory.RegisterType. Something like we have in our tests for DateTime:

nhibernate-core/src/NHibernate.Test/TypesTest/ChangeDefaultTypeFixture.cs

Line 30 in 322de5e

TypeFactory.RegisterType(typeof(DateTime), _testDefaultDateTimeType, TypeFactory.EmptyAliases);