SqliteDataReader.GetDateTimeOffset() UTC offset error

[DanielGekeler]

Bug description

Reading a DateTime field from a SQLite Database using SqliteDataReader.GetDateTimeOffset() returns a wrong time, ignoring UTC.

DateTime is always stored as UTC in SQLite Databases (see https://sqlite.org/lang_datefunc.html) but SqliteDataReader.GetDateTimeOffset() interprets it as local time.

When running the provided program, the row in the Database will contain a ISO-8601 string with the current time in UTC as expected. However in the output you will see the exact same time but with a timezone indicator of your local timezone. This is incorrect as the offset would need to be +00:00 for the times to be equal.

Fixing this bug is a breaking change but it should have never been like that anyways.

Your code

using Microsoft.Data.Sqlite;

using var sqlite = new SqliteConnection("Data Source=:memory:");
sqlite.Open();

using var command1 = sqlite.CreateCommand();
command1.CommandText = """
CREATE TABLE exampleTable (date DATETIME);
INSERT INTO exampleTable (date) VALUES (datetime('now'));
""";
command1.ExecuteNonQuery();

using var command2 = sqlite.CreateCommand();
command2.CommandText = "SELECT rowid, date FROM exampleTable";

var reader = command2.ExecuteReader();
while (reader.Read())
{
    var dateString = reader.GetString(reader.GetOrdinal("date"));
    // Actual string in the database
    Console.WriteLine(dateString);

    var dateTimeOffest = reader.GetDateTimeOffset(reader.GetOrdinal("date"));
    // how it is parsed as local time (wrong)
    Console.WriteLine(dateTimeOffest.ToString("O"));

    // does not match the database after applying the zone offset
    Console.WriteLine(dateTimeOffest.ToUniversalTime().ToString("O"));
}

Stack traces

Microsoft.Data.Sqlite version

9.0.5

Target framework

.NET 8.0

Operating system

Arch Linux

[roji]

DateTime is always stored as UTC in SQLite Databases (see https://sqlite.org/lang_datefunc.html)

I'm no SQLite expert, but there's this sentence in the same doc page:

Formats 2 through 10 may be optionally followed by a timezone indicator of the form "[+-]HH:MM" or just "Z". The date and time functions use UTC or "zulu" time internally, and so the "Z" suffix is a no-op. Any non-zero "HH:MM" suffix is subtracted from the indicated date and time in order to compute zulu time.

This seems to indicate that SQLite does support storing non-UTC timestamps. Of course, this doesn't mean that SqliteDataReader.GetDateTimeOffset() doesn't have a bug - just pointing out this apparent inaccutacy.

[DanielGekeler]

This seems to indicate that SQLite does support storing non-UTC timestamps. Of course, this doesn't mean that SqliteDataReader.GetDateTimeOffset() doesn't have a bug - just pointing out this apparent inaccutacy.

Fair.

A DATETIME field without a zone indicator implies UTC. SqliteDataReader.GetDateTimeOffset() assumes local time.

[cincuranet]

This also affects writing parameter values of type DateTimeOffset into database. Will fix that as well.

[DanielGekeler]

After taking a closer look, I noticed that GetDateTime() for regular DateTime has the same bug. Like you mentioned, parameter writing is probably effected as well.

Could you fix that as well?

[cincuranet]

We're discussing DateTime now. Could you share what do you feel is incorrect?

[DanielGekeler]

The Kind property of the resulting DateTime when using SqliteDataReader.GetDateTime() is Unspecified which will lead to conversion errors. You can test it with the code example I shared at the beginning of this issue.

[cincuranet]

We discussed this exactly. For timestamps with time zone, when read using GetDateTime, we'll return it as Utc. For the rest we think Unspecified is more correct. For storing DateTime, we ignore DateTimeKind and don't do any conversion (similar to what i.e. SqlClient is doing). The DateTime is fundamentally limited in expressing what the time actually means and we don't want to introduce magic conversions. If person cares about the time and zones, DateTimeOffset is superior to DateTime.

[DanielGekeler]

For storing DateTime, we ignore DateTimeKind and don't do any conversion

If it's a DateTime with a DateTimeKind of UTC, it is stored that way. If it has a DateTimeKind of Local it is stored like it is. Without a zone indicator (Yes, really). You are absolutely right, if you read the value again, it must have a DateTimeKind of Unspecified because you have no idea what you just got from your database. Could be anything. Maybe it was a 42 bit unsigned Boeing 747.

The DateTime is fundamentally limited in expressing what the time actually means and we don't want to introduce magic conversions.

I deeply hate the DateTime class and I hope whoever made it regrets it. That class has caused me so much trouble.

I understand your position and that you don't want to introduce further complexity into this. All it takes is just one unknowing developer on a team and all DATETIME fields of a database are messed up.

Please consider formatting the DateTime with a zone indicator, if it has a DateTimeKind of Local. I think that change would actually make it easier to understand. Would there be any downsides of this?

[cincuranet]

Would there be any downsides of this?

It would be a breaking change.