Pomelo.entityframeworkcore.mysql: Mariadb Datetime vs. Datetime(6)

[klyse]

Steps to reproduce

Use MariaDb version 10.3.12:

options.ServerVersion(new Version(10, 3, 12), ServerType.MariaDb);

https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/blob/3c01ad48f1dfa921a1a528e944a069ab2680d2a8/src/EFCore.MySql/Storage/Internal/MySqlTypeMappingSource.cs#L299-L305

https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/blob/3c01ad48f1dfa921a1a528e944a069ab2680d2a8/src/EFCore.MySql/Storage/Internal/ServerVersion.cs#L50

As seen in the two code snippets there is no possibility to use datetime as default type instead of datetime(6).

Is there a specific reason why it is not possible to use datetime as default?

Further technical details

MariaDb version: 10.3.12-MariaDB
Operating system: CentOs 7
Pomelo.EntityFrameworkCore.MySql version: 2.2.0
Microsoft.AspNetCore.App version: netcoreapp2.2

[crozone]

I believe if you use the annotation [Column(TypeName = "DATETIME")] on the property, it will override this.

[klyse]

Okay. I’m aware of this possibility.
Is there no other way? For example default setting? It would be nice to avoid the situation where every single DateTime must be flagged with this attribute.

[lauxjpn]

We should be able to provide an options for that in 3.0.0.
Maybe something like DefaultDateTimeMapping or DefaultDateTimeDataType, with an Enum to choose the type from? The current behavior would still be the default, but it would then be possible to override it on a DbContext basis.

[crozone]

We should be able to provide an options for that in 3.0.0.
Maybe something like DefaultDateTimeMapping or DefaultDateTimeDataType, with an Enum to choose the type from? The current behavior would still be the default, but it would then be possible to override it on a DbContext basis.

This would be really useful since using "TIMESTAMP" or "TIMESTAMP(6)" for a DateTime/DateTimeOffset column also appears to be a common usecase.

[lauxjpn]

Will simple direct MySqlOptions suffice, or should this get refactored into its own DataTypeMappings class (containing the Enums), with the options implementing an extension method like DataTypeMappings(DataTypeMappings mappings)?

[lauxjpn]

The self contained DataTypeMappings class is probably the way to go here, where we can also move the out-of-the-box Boolean to bit or tinyint conversion option (that should be revisited).
See #654, #577

[lauxjpn]

Also, @bgrainger suggested to use tinyint(1) as to default database column type for the Boolean CLR type, as tinyint(1) shows a much more consistent and deterministic behavior in MySQL than bit(1).

See https://github.com/mysql-net/MySqlConnector/issues/708 and https://github.com/mysql-net/MySqlConnector/issues/713 for recent problems regarding bit(1) columns.

[lauxjpn]

This concrete issue does not get resolved by #843, because usage of datetime(6) is supported by MariaDB since 10.1.2, @klyse uses 10.3.12 and #847 corrects the MariaDB support.
To solve this particular issue, the DataTypeMappings class is still needed.

I will implement this feature for 3.0.0-rc2.

[lauxjpn]

@bgrainger Am I correct in assuming that with TreatTinyAsBoolean=True, all tinyints (independent of their declared display width) will be treated as System.Boolean by MySqlConnector?

For example, tinyint(1) and tinyint(3) would both be treated as System.Boolean?

Update:
Looking into this, it seems as MySQL is still just using bool as a synonym for tinyint(1), so MySqlConnector is probably doing the same.
As display widths are deprecated now for numeric types, this is likely to change in the future. My guess is, that bool will then just be a synonym for tinyint because the display width has really no storage effect anyway.

[lauxjpn]

This is fixed by #909. Take a look a the PR description for a quick example how to use the new feature.

[bgrainger]

Am I correct in assuming that with TreatTinyAsBoolean=True, all tinyints (independent of their declared display width) will be treated as System.Boolean by MySqlConnector?

For example, tinyint(1) and tinyint(3) would both be treated as System.Boolean?

No; only TINYINT(1) (not UNSIGNED) is treated as System.Boolean.

Note that in MySQL 8.0.17, using TINYINT(1) in a CREATE TABLE statement will result in Warning (Code 1681): Integer display width is deprecated and will be removed in a future release.. Using BOOL gives no warning but still creates a TINYINT(1) column (which is compatible with MySqlConnector).

If MySQL Server ever deprecates the _storage_ of column width (e.g., in a column definition packet) then we'll be in trouble as we won't be able to tell BOOL apart from any other TINYINT column. I haven't seen any suggestion that this will happen, though.

[lauxjpn]

As far as I understand, tinyint(1) does not contain a column width of 1, but a display width of 1. Are display widths being saved in the same location that column sizes/widths are?

Since display widths are going away for numeric types in the future, we would just need to make sure, that tinyint columns can be retrieved as both System.Boolean and System.SByte, depending on what method is being called to get the value.

I don't think this is going to be a problem for us, just something to keep in the back of our heads.

[bgrainger]

All tinyint values take one byte of storage. Even though MySQL appears to have deprecated the tinyint(1) syntax, creating a bool column still stores a 1 in the column length field (in the table metadata). I believe this is the same metadata field where (for example) char(30) or binary(10) stores its column length; at least it's returned in the same field when column metadata is returned for a result set.

[bgrainger]

Note that in MySQL 8.0.17, using TINYINT(1) in a CREATE TABLE statement will result in Warning

This is still true in MySQL 8.0.19.

Since display widths are going away for numeric types in the future

Looks like an exception is being made for TINYINT(1). From https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-19.html#mysqld-8-0-19-feature:

Display width specification for integer data types was deprecated in MySQL 8.0.17, and now statements that include data type definitions in their output no longer show the display width for integer types, with these exceptions:

• The type is TINYINT(1). MySQL Connectors make the assumption that TINYINT(1) columns originated as BOOLEAN columns; this exception enables them to continue to make that assumption.

[lauxjpn]

@bgrainger Thanks for the heads-up! It's convenient, that they keep the TINYINT(1) handling as is for now, but I am surprised that they already making the move to get rid of the integer type display widths, since they just deprecated them quite recently.