Skip to content
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.

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

npgsql 6 breaks after DateTime migration: "year must be between 1 and 9999" #2177

Closed
qwertie opened this issue Dec 21, 2021 · 11 comments
Closed

Comments

@qwertie
Copy link

qwertie commented Dec 21, 2021

After following the date migration guide to change types from "without time zone" to "with time zone", I'm still having multiple errors. One of them occurs when reading rows from three of our tables:

Out of the range of DateTime (year must be between 1 and 9999)
   at Npgsql.Internal.TypeHandlers.DateTimeHandlers.DateTimeUtils.ReadDateTime(NpgsqlReadBuffer buf, DateTimeKind kind)
   at Npgsql.Internal.TypeHandlers.DateTimeHandlers.TimestampTzHandler.Read(NpgsqlReadBuffer buf, Int32 len, FieldDescription fieldDescription)
   at Npgsql.Internal.TypeHandling.NpgsqlTypeHandler.Read[TAny](NpgsqlReadBuffer buf, Int32 len, FieldDescription fieldDescription)
   at Npgsql.NpgsqlDataReader.GetFieldValue[T](Int32 ordinal)
   at Npgsql.NpgsqlDataReader.GetDateTime(Int32 ordinal)
   at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at BarreleyeWebTest.BarreleyeIntegrationTestBase.<PrepareTestDatabase>b__17_0(BarreleyeDbContext context) in C:\Dev\BarreleyeServer\BarreleyeWebTest\BarreleyeIntegrationTestBase.cs:line 69

Inner exception:

Ticks must be between DateTime.MinValue.Ticks and DateTime.MaxValue.Ticks. (Parameter 'ticks')
   at System.DateTime.ThrowTicksOutOfRange()
   at System.DateTime..ctor(Int64 ticks, DateTimeKind kind)
   at Npgsql.Internal.TypeHandlers.DateTimeHandlers.DateTimeUtils.DecodeTimestamp(Int64 value, DateTimeKind kind)
   at Npgsql.Internal.TypeHandlers.DateTimeHandlers.DateTimeUtils.ReadDateTime(NpgsqlReadBuffer buf, DateTimeKind kind)

My database contains some dates that I think were set to 0001/01/01 00:00:00, but which BeeKeeper (incorrectly) reports as 0001/12/31... and this breaks npgsql v6. As a workaround I have executed a series of queries like this to fix the problem:

UPDATE table_name
SET last_updated = date '0001-01-02'
WHERE last_updated < date '0001-01-02'

I think the problem can be fixed by changing this function in Npgsql\Internal\TypeHandlers\DateTimeHandlers\DateTimeUtils.cs:

    internal static DateTime DecodeTimestamp(long value, DateTimeKind kind)
        => new(value * 10 + PostgresTimestampOffsetTicks, kind);

do instead:

    internal static DateTime DecodeTimestamp(long value, DateTimeKind kind)
        => new(Math.Max(0, value * 10 + PostgresTimestampOffsetTicks), kind);

or maybe even:

    internal static DateTime DecodeTimestamp(long value, DateTimeKind kind)
        => new(Math.Max(0, Math.Min(DateTime.MaxValue.Ticks, value * 10 + PostgresTimestampOffsetTicks)), kind);

The other error I'm getting occurs when I create a fresh database by calling Migrate().

@roji
Copy link
Member

roji commented Dec 21, 2021

@qwertie your proposed suggestion with Math.Max would silently read PostgreSQL timestamps from before the year 0 to DateTime.MinValue (which is January 1, 0001 at 00:00:00.000). This is not something we should do - it would yield incorrect data.

Can you provide full information on your problem, and how I'd reproduce this? Is your column timestamp without time zone or timestamp with time zone? What exact value do you have in there? At the end of the day it seems like you have a negative timestamp in your database, in which case the above behavior is correct...

@qwertie
Copy link
Author

qwertie commented Dec 21, 2021

I'm not actually sure how to reproduce. I thought that the 0001/01/01 values were the result of adding the last_updated columns after the DB was already full of data, but the EF Core migration says to use a default of 2020 which should have avoided this problem:

        migrationBuilder.AddColumn<DateTime>(
            name: "last_updated",
            table: "entities",
            type: "timestamp without time zone",
            nullable: false,
            defaultValue: new DateTime(2020, 1, 1, 0, 0, 0, 0, DateTimeKind.Utc));

Looking at it now, yes, lots of rows have values of "2019-12-31 17:00:00-07" - and I'm not sure, can you tell me if the timezone "-07" is physically stored in the database or if Beekeeper just prefers to show local time?

So I'm not sure how those values got in there, but they must have been there as a result of storing ordinary default(DateTime) values, because we don't use any custom SQL or stored procs (except MigrationBuilder.Sql which was not used for these columns).

@roji
Copy link
Member

roji commented Dec 21, 2021

Looking at it now, yes, lots of rows have values of "2019-12-31 17:00:00-07" - and I'm not sure, can you tell me if the timezone "-07" is physically stored in the database or if Beekeeper just prefers to show local time?

The offset (-07) isn't stored in the database; the timestamp with time zone type stores UTC in the database, and only displays it in the local time zone when displayed as text (using the PostgreSQL TimeZone parameter as the local time zone). If you don't want to see these local representations (of the UTC timestamp stored in the database), set TimeZone to UTC either on your specific connection or in your postgresql.conf (at which point it will apply globally).

However, I think there may be some confusion here: PostgreSQL only shows a string representation with an offset (2019-12-31 17:00:00-07 above) for timestamp with time zone, and not for timestamp without time zone; the former is meant to represent UTC timestamps, while the latter is meant to represent unspecified/local timestamps.

Now, when following the migration guide, did you also take care to add SET TimeZone='UTC' to your migration as detailed there? If not, please reread those notes carefully as this is tricky stuff indeed...

I hope the above sheds some light. If you need more help, I need to know what the actual column type is in the database (timestamp with time zone or timestamp without time zone), what data was there before the migration (UTC timestamps? Something else?) and exactly how you did the migration.

@qwertie
Copy link
Author

qwertie commented Dec 21, 2021

did you also take care to add SET TimeZone='UTC' to your migration...?

Yes, and I believe 2019-12-31 17:00:00-07 equals 2020-01-01 00:00:00Z, so the migration appears to have worked correctly,

Pre-migration I was using UTC timestamps stored in default-typed DateTime fields, and as you can see above, the migration generator was choosing "timestamp without time zone" for this. Edit: Also, our generated UpgradeNpgsqlToV6 migration is full of code like this:

        migrationBuilder.AlterColumn<DateTime>(
            name: "last_updated",
            table: "entities",
            type: "timestamp with time zone",
            nullable: false,
            oldClrType: typeof(DateTime),
            oldType: "timestamp without time zone");

"UTC-ness" was enforced using conversions like this:

    entity.Property(nameof(LastUpdated))
        .HasConversion<DateTime, DateTime>(
            d => d.ToUniversalTime(), d => new DateTime(d.Ticks, DateTimeKind.Utc));

where HasConversion means

    public static PropertyBuilder HasConversion<ModelT, DBT>(this PropertyBuilder property,
        Expression<Func<ModelT, DBT>> onSave, Expression<Func<DBT, ModelT>> onLoad, 
        ConverterMappingHints? hints = null)
        => property.HasConversion(new ValueConverter<ModelT, DBT>(onSave, onLoad, hints));

Just before or after the migration (can't remember, but it shouldn't matter), I changed the HasConversion code to

        .HasConversion<DateTime, DateTime>(d => d.ToUniversalTime(), d => d);

So the puzzle is how it's possible that some default(DateTime) values were stored in such a way that everything was working pre-migration but the server is broken post-migration.

@roji
Copy link
Member

roji commented Dec 21, 2021

So can you confirm you have negative timestamps in the database? If so, what is their exact value (preferably in UTC)? That might shed some light on what happened.

@qwertie
Copy link
Author

qwertie commented Dec 21, 2021

Thanks for your responsiveness! Sadly I "blew away" the exact timestamps as part of the workaround mentioned above. I have a backup of the non-migrated production database that I have now experimented with... and I cannot reproduce the problem even though it does contain dates in year 0001:

select last_updated, EXTRACT(EPOCH FROM last_updated) from entities
...
2020-01-01 00:00:00    1577836800
2020-01-01 00:00:00    1577836800
0001-01-01 00:00:00    -62135596800
0001-01-01 00:00:00    -62135596800
...

-- where 1970 is 0
select * FROM EXTRACT(EPOCH FROM TIMESTAMP '1970-01-01 00:00:00') 
0

As expected, running the query post-migration gives the same integers, and the app works, though Beekeeper's output is odd:

select last_updated, EXTRACT(EPOCH FROM last_updated) from entities
...
2019-12-31 17:00:00-07             1577836800
2019-12-31 17:00:00-07             1577836800
0001-12-31 17:00:04-06:59:56 BC    -62135596800
0001-12-31 17:00:04-06:59:56 BC    -62135596800
...

And if I use my code to create new rows in the upgraded table with their default .NET value, again it is displayed wrong but the number makes sense, as it is 7 hours after default(DateTime), corresponding to default(DateTime).ToUniversalTime() (0001-01-01 07:00 UTC).

0001-01-01 00:00:04-06:59:56    -62135571600

I am closing as not reproducible.

@qwertie qwertie closed this as completed Dec 21, 2021
@roji
Copy link
Member

roji commented Dec 22, 2021

@qwertie ok. Re the display issue, keep in mind that as I wrote above, that should just be a matter of your PostgreSQL TimeZone parameter. If you set that to UTC (e.g. in your postgresql.conf), Beekeeper (and any other text-based PostgreSQL UI) should start showing timestamp with time zone values in UTC instead.

@qwertie
Copy link
Author

qwertie commented Dec 22, 2021

Yeah, I couldn't figure out how to make that work. A SET TimeZone='UTC' by itself doesn't impact other queries in Beekeeper, and if I put it above any other query then it says "Query executed successfully. No results".

@roji
Copy link
Member

roji commented Dec 22, 2021

@qwertie you can set it in your PostgreSQL's postgresql.conf, that would make it the default value for all connections. Otherwise maybe there's a way to tell Beekeeper to set a parameter for its connections...

@TriHyde
Copy link

TriHyde commented Dec 24, 2022

Can I reopen this?

I have old data in my database which causes a crash after upgrading to npgsql 6.

An example of a timestamp in the database (which the pgAdmin can read just fine) is "0001-01-01 00:00:00+01:39:49" (in pgAdmin view). pgAdmin does not crash but npgsql does, since it cannot READ that timestamp without throwing this particular exeption: "year must be between 1 and 9999", inner: "Ticks must be between DateTime.MinValue.Ticks and DateTime.MaxValue.Ticks. (Parameter 'ticks')".

Thanks!

@roji
Copy link
Member

roji commented Dec 24, 2022

@TriHyde your case seems to be the same as @qwertie - please see the discussion above. You have negative timestamp data in your database, and that's simply not representable with .NET DateTime. I recommend fixing up your data; for example, you could convert those values to -infinity with a single UPDATE command; after that, Npgsql will read those values as DateTime.MinValue.

If that isn't suitable, can you please provide more on context on why, and how exactly you're using those values?

@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Dec 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants