DataObjects-NET
Support Npgsql 6.0+
A lot of tests are failed. At least DateTime and DateTimeOffset support is broken for all the test instances from v9.6 up to v13, basic INSERTs are broken. I cannot merge this PR.
A lot of tests are failed. At least DateTime and DateTimeOffset support is broken for all the test instances from v9.6 up to v13, basic INSERTs are broken. I cannot merge this PR.
Yes, new version of Npgsql driver throw exception if you try save DateTime without KindType.Utc to DB column with support timezone info.
Offical Postgres doc (https://www.npgsql.org/doc/types/datetime.html) contains information about this and suggest several solutions:
- Enable special static option for application: AppContext.SetSwitch("Npgsql.EnableLegacyTimestampBehavior", true);
- Fix all DateTime and set it right KindType
Currently all test use DateTime with KindType.Unspecified.
After enable options for support legacy behavior all test successful.
I sugest change KindType for DateTime property on UTC when parsing of test data from XML and for others instance of DateTime , it's ok for you?
Enable special static option for application: AppContext.SetSwitch("Npgsql.EnableLegacyTimestampBehavior", true);
It worked fine before so having another option to turn on to not screw application work is bad idea.
I suggest change KindType for DateTime property on UTC
Imagine you had DateTime values in database, you updated the package and all dates that had been stored before became UTC dates. That's a nightmare.
For DateTime values I suggest to use timestamp without timezone explicitly. Apparently, Npgsql uses timestamp with timezone for that by default and had broken backwards compatibility (what an idea!🤦♂️). But this is half of the problem, the second part is DateTimeOffset values. It should work too. I would try to switch zone for values to UTC before saving (PgSQL did it anyway on server side).
To do so take a look at TypeMapper.cs which is in all of SQL drivers. For each supported .Net type it a has trinity of methods:
-
MapXXX- maps .net type to native SQL type in form ofSqlValueType; -
BindXXX- binds value of .Net type to DbParameter -
ReadXXX- reads values from query result row and transforms it to corresponding .Net type.
You need to change or override BindDateTime() and BindDateTimeOffset(), the second one is already overridden for sure.
If you have any questions, please ask.
By the way, what was the problem with IsTransactionCompleted()?
@alex-kulakov fixed mapping for DateTimeOffset and DateTime. All tests passed in master branch same way passed with new driver version in current branch
IsTransactionCompleted() use private field IsCompleted for get transaction state in early versions of npgsql driver it field was be public. Now Npgsql checks what the transaction completed internally in methods Commit() and Rollback()
@PopovKS , that's weird that you fixed DateTimeOffset in v8_0.TypeMapper and DateTime - in v10_0.TypeMapper? That means no proper support for DateTime values for PostgreSQL v9.6 which is still supported PostgreSQL version.
You don't need to override all three Map, Read, Bind methods, As I understand from my quick investigation something like this
public override void BindDateTime(DbParameter parameter, object value)
{
var nativeParameter = (NpgsqlParameter) parameter;
nativeParameter.NpgsqlValue = value ?? DBNull.Value;
}
works for all DateTime values (Unspecified and Utc kinds). Funny, huh? 😁 I did it in v8_0.TypeMapper, so no need to change v10_0.TypeMapper at all. I'll test the code above through all test storages we usually test on during today and tomorrow. Maybe some additional changes will have to be made.
IsTransactionCompleted() use private field IsCompleted for get transaction state in early versions of npgsql driver it field was be public. Now Npgsql checks what the transaction completed internally in methods Commit() and Rollback()
Got it.
There is another thing that should be investigated - DateTime.Min and DateTime.Max replacing with Infinity. Maybe there are more things they wanted to break compatibility.