Skip to content

Prevent SQLite from messing with timezone offset of DateTime. #1374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Prevent SQLite from messing with timezone offset of DateTime. #1374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/NHibernate/Driver/SQLite20Driver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Data;
using System.Data.Common;

Expand Down Expand Up @@ -57,6 +58,23 @@ private static void Connection_StateChange(object sender, StateChangeEventArgs e
}
}

public override void AdjustCommand(DbCommand command)
{
ChangeDateTimeKindToUnspecified(command.Parameters);
}

private static void ChangeDateTimeKindToUnspecified(DbParameterCollection commandParameters)
{
for (var i = 0; i < commandParameters.Count; i++)
{
var commandParameter = commandParameters[i];
if (commandParameter.Value is DateTime dateTimeParameter)
{
commandParameter.Value = DateTime.SpecifyKind(dateTimeParameter, DateTimeKind.Unspecified);
}
}
}

public override bool UseNamedPrefixInSql
{
get { return true; }
Expand Down
2 changes: 1 addition & 1 deletion src/NHibernate/Type/AbstractDateTimeType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ protected AbstractDateTimeType(SqlType sqlTypeDateTime) : base(sqlTypeDateTime)
/// <param name="dateValue">The <see cref="System.DateTime" /> to adjust.</param>
/// <returns>A <see cref="System.DateTime" />.</returns>
protected virtual DateTime AdjustDateTime(DateTime dateValue) =>
Kind == DateTimeKind.Unspecified ? dateValue : DateTime.SpecifyKind(dateValue, Kind);
DateTime.SpecifyKind(dateValue, Kind);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is really this change required? SQLite users working with DateTimeKind=UTC would have a legitimate grievance with this change, as it would wreck their reading of dates, causing them to be unspecified instead of being said as UTC, and forcing them to use NHibernate Utc types instead.

I do not think we should force unspecified unless really having a compelling reason to do it. I think the NHibernate "unspecified" types should have as semantic "no change at all to read date kind", as was doing TimestampType previously (which is now become DateTimeType.) The fact that previous DateTimeType, cutting fractional seconds, is losing the kind, was likely accidental design. (DateTimeNoMsType probably still does that indeed.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should stay the way it is without any configuration. See the unit test I created and my comment on the issue.


/// <inheritdoc />
public override object Get(DbDataReader rs, int index, ISessionImplementor session) =>
Expand Down