Ignore zerofill in MySqlTypeMappingSource

[jespersh]

Steps to reproduce

1. Install the latest EFCorePowerTools build: http://vsixgallery.com/extensions/f4c4712c-ceae-4803-8e52-0e2049d5de9f/extension.vsix (linked taken from https://github.com/ErikEJ/EFCorePowerTools Readme) into Visual Studio
2. Install the MySQL for Visual Studio provider: https://dev.mysql.com/downloads/windows/visualstudio/
3. Create a table with a zerofill column

CREATE TABLE `testzerofilltable` (
  `id` INT(10) UNSIGNED NOT NULL AUTO_INCREMENT,
  `missingcolumn` INT(3) UNSIGNED ZEROFILL NOT NULL DEFAULT '000',
  PRIMARY KEY (`id`)
) ENGINE=INNODB DEFAULT CHARSET=latin1

4. Use the Reverse Engineer part of the EFCorePowerTools.
5. See the property of the column is missing:

using System;
using System.Collections.Generic;

namespace TestProject
{
    public partial class Testzerofilltable
    {
        public uint Id { get; set; }
    }
}

The issue

After finishing work on supporting mysql reverse engineering to generate entity files here: ErikEJ/EFCorePowerTools#17 - I discovered our columns with int(3) unsigned zerofill weren't being generated as properties.

Appears to be missing from this file: https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/blob/master/src/EFCore.MySql/Storage/Internal/MySqlTypeMappingSource.cs#L110

The property would have been generated here: https://github.com/aspnet/EntityFrameworkCore/blob/master/src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs#L196

Further technical details

MySQL version: 5.5.45
Operating system: Windows 10
Pomelo.EntityFrameworkCore.MySql version: 2.2.0
Microsoft.AspNetCore.App version: Not used

Other details about my project setup: Used https://github.com/ErikEJ/EFCorePowerTools to reverse engineer a database

[mguinness]

Zerofill is non-standard and is only used for display purposes within apps like MySQL Workbench, but serves no purpose in EF Core other than to specify a field is unsigned.

The scaffold code in this library could be modified to strip the zerofill and replace with unsigned (if not already specified). If someone would like to create a PR for that purpose then we'll review it for inclusion.

Pomelo.EntityFrameworkCore.MySql/src/EFCore.MySql/Scaffolding/Internal/MySqlDatabaseModelFactory.cs

Lines 144 to 145 in c58e00c

	StoreType = Regex.Replace(reader.GetString(1), @"(?<=int)\(\d+\)(?=\sunsigned)",
	string.Empty),

There was also a similar issue in the upstream RelationalTypeMapper class that meant stripping precision from unsigned fields to allow mapping.

[lauxjpn]

The current MySQL docs state the following:

As of MySQL 8.0.17, the ZEROFILL attribute is deprecated for numeric data types, as is the display width attribute for integer data types. Support for ZEROFILL and display widths for integer data types will be removed in a future MySQL version. Consider using an alternative means of producing the effect of these attributes. For example, applications could use the LPAD() function to zero-pad numbers up to the desired width, or they could store the formatted numbers in CHAR columns.

I will therefore close this issue.

[jespersh]

Can I still provide a pull request? For reasons our customers are on 5.5 and up on the beat

[lauxjpn]

I don't see any issues with adding this feature to the 2.2-maint branch, so go ahead.
We should not implement support for this feature in 3.0.0 however. Therefore, this will lead to a breaking change for anybody, who will start using this feature on 2.2 and later upgrade to 3.0.0.

[jespersh]

Okay, so I finally got 2.2-maint working together with EFCorePowerTools that I am working with. And there's two ways I could correct this.
I could strip out Zerofill in

Pomelo.EntityFrameworkCore.MySql/src/EFCore.MySql/Scaffolding/Internal/MySqlDatabaseModelFactory.cs

Lines 144 to 145 in c58e00c

	StoreType = Regex.Replace(reader.GetString(1), @"(?<=int)\(\d+\)(?=\sunsigned)",
	string.Empty),

or I could add

{ "tinyint unsigned zerofill", _utinyint },
{ "smallint unsigned zerofill", _usmallint },
{ "mediumint unsigned zerofill", _uint },
{ "int unsigned zerofill", _uint },
{ "bigint unsigned zerofill", _ubigint },

to

Pomelo.EntityFrameworkCore.MySql/src/EFCore.MySql/Storage/Internal/MySqlTypeMappingSource.cs

Lines 124 to 134 in c58e00c

	// integers
	{ "tinyint", _tinyint },
	{ "tinyint unsigned", _utinyint },
	{ "smallint", _smallint },
	{ "smallint unsigned", _usmallint },
	{ "mediumint", _int },
	{ "mediumint unsigned", _uint },
	{ "int", _int },
	{ "int unsigned", _uint },
	{ "bigint", _bigint },
	{ "bigint unsigned", _ubigint },

Which would be preferred?

[lauxjpn]

If this is just about reverse engineering, then stripping it from the store type, as already done with unsigned, will probably be the simplest solution.

We did improve this part recently for 3.0.0 with #896, but for 2.2.6, just stripping it away should be fine.