Hangfire.PostgreSql icon indicating copy to clipboard operation
Hangfire.PostgreSql copied to clipboard

Remaining issues with `timestamptz` columns

Open azygis opened this issue 3 years ago • 8 comments

Hello,

I am still receiving a timestampz related error after upgrading to 1.9.6 when trying to delete items from the Hangfire Dashboard.

2022-02-15T17:08:32.605883580Z System.InvalidCastException: Cannot write DateTime with Kind=UTC to PostgreSQL type 'timestamp without time zone', consider using 'timestamp with time zone'. Note that it's not possible to mix DateTimes with different Kinds in an array/range. See the Npgsql.EnableLegacyTimestampBehavior AppContext switch to enable legacy behavior.
2022-02-15T17:08:32.605895920Z    at Npgsql.Internal.TypeHandlers.DateTimeHandlers.TimestampHandler.ValidateAndGetLength(DateTime value, NpgsqlParameter parameter)
2022-02-15T17:08:32.605957028Z    at Npgsql.Internal.TypeHandlers.DateTimeHandlers.TimestampHandler.ValidateObjectAndGetLength(Object value, NpgsqlLengthCache& lengthCache, NpgsqlParameter parameter)
2022-02-15T17:08:32.605964475Z    at Npgsql.NpgsqlParameter.ValidateAndGetLength()
2022-02-15T17:08:32.605967852Z    at Npgsql.NpgsqlParameterCollection.ValidateAndBind(ConnectorTypeMapper typeMapper)
2022-02-15T17:08:32.605970993Z    at Npgsql.NpgsqlCommand.ExecuteReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken)
2022-02-15T17:08:32.605974164Z    at Npgsql.NpgsqlCommand.ExecuteReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken)
2022-02-15T17:08:32.605977315Z    at Npgsql.NpgsqlCommand.ExecuteNonQuery(Boolean async, CancellationToken cancellationToken)
2022-02-15T17:08:32.605980469Z    at Npgsql.NpgsqlCommand.ExecuteNonQuery()
2022-02-15T17:08:32.605983610Z    at Dapper.SqlMapper.ExecuteCommand(IDbConnection cnn, CommandDefinition& command, Action`2 paramReader) in /_/Dapper/SqlMapper.cs:line 2848
2022-02-15T17:08:32.605987193Z    at Dapper.SqlMapper.ExecuteImpl(IDbConnection cnn, CommandDefinition& command) in /_/Dapper/SqlMapper.cs:line 581
2022-02-15T17:08:32.605990555Z    at Dapper.SqlMapper.Execute(IDbConnection cnn, String sql, Object param, IDbTransaction transaction, Nullable`1 commandTimeout, Nullable`1 commandType) in /_/Dapper/SqlMapper.cs:line 452
2022-02-15T17:08:32.605994030Z    at Hangfire.PostgreSql.PostgreSqlDistributedLock.TransactionLockHandler.Lock(String resource, TimeSpan timeout, IDbConnection connection, PostgreSqlStorageOptions options)```


I have gone so far as to delete all the hangfire tables in my DB and let them be re-created to attempt to resolve (in case a new migration was involved) but no luck there.

_Originally posted by @stephen776 in https://github.com/frankhommers/Hangfire.PostgreSql/issues/234#issuecomment-1040550306_

azygis avatar Feb 15 '22 17:02 azygis

@stephen776 While we solve that, I suggest adding the compatibility flag, this restores the previous Npgsql handling (in case you haven't yet). Of course, it should be fixed sooner than later here, but at least for now it should be okay to be with that flag.

azygis avatar Feb 15 '22 17:02 azygis

Thanks! Is there currently an open issue for this that I can track?

On Feb 15, 2022 at 12:27:04 PM, Žygimantas A. @.***> wrote:

@stephen776 https://github.com/stephen776 While we solve that, I suggest adding the compatibility flag https://www.npgsql.org/doc/release-notes/6.0.html#opting-out-of-the-new-timestamp-mapping-logic, this restores the previous Npgsql handling (in case you haven't yet). Of course, it should be fixed sooner than later here, but at least for now it should be okay to be with that flag.

— Reply to this email directly, view it on GitHub https://github.com/frankhommers/Hangfire.PostgreSql/issues/246#issuecomment-1040562799, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEJ7LYSBCSNFZG64TXD6KTU3KEGRANCNFSM5OPGCWVQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

stephen776 avatar Feb 16 '22 13:02 stephen776

Literally this one. Created this issue from your comment on merged request.

azygis avatar Feb 16 '22 14:02 azygis

Any updates to report?

stephen776 avatar Mar 30 '22 14:03 stephen776

No, we are hoping someone can dive into this issue, fix it and send a PR.

frankhommers avatar Mar 30 '22 19:03 frankhommers

Just checking back in. Has there been any progress on this?

I’m trying to determine if this is still an issues or a problem with my implementation

stephen776 avatar May 29 '22 01:05 stephen776

No, we are hoping someone can dive into this issue, fix it and send a PR.

frankhommers avatar May 29 '22 10:05 frankhommers

Since you use DateTime.UtcNow (i.e. DateTimeKind.Utc) in the library, therefore database columns of the type "timestamp without time zone" need to be migrated to the type "timestamp with time zone". https://www.npgsql.org/doc/release-notes/6.0.html#migrating-columns-from-timestamp-to-timestamptz

In the above case, there is a conflict between using DateTime.UtcNow here: https://github.com/frankhommers/Hangfire.PostgreSql/blob/master/src/Hangfire.PostgreSql/PostgreSqlDistributedLock.cs#L138 and having the column of type "timestamp without time zone" here: https://github.com/frankhommers/Hangfire.PostgreSql/blob/master/src/Hangfire.PostgreSql/Scripts/Install.v7.sql#L16

Link to the code, where the exception is thrown: https://github.com/npgsql/npgsql/blob/main/src/Npgsql/Internal/TypeHandlers/DateTimeHandlers/TimestampHandler.cs#L42

psupina avatar Aug 31 '22 13:08 psupina

Link to the code, where the exception is thrown: https://github.com/npgsql/npgsql/blob/main/src/Npgsql/Internal/TypeHandlers/DateTimeHandlers/TimestampHandler.cs#L42

Conclusion: Npgsql demands this and it would be better to convert all timestamp to timestamptz columns.

Still someone has to do it ;-)

frankhommers avatar Oct 25 '22 15:10 frankhommers