Pomelo.entityframeworkcore.mysql: CanBeNull, but NotNull

[abelevtsov]

Hello!

There are misleading attributes for errorNumbersToAdd parameter:

• CanBeNull here
  https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/blob/95b34f4e8367c4f221acf7b221a850e38d7e5795/src/EFCore.MySql/MySqlRetryingExecutionStrategy.cs#L76
• and NotNull here
  https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/blob/9be87311c943ffd1389052a05cef4197af991937/src/EFCore.MySql/Infrastructure/MySqlDbContextOptionsBuilder.cs#L74

So we're forced to pass argument like this (when c# nullable reference types feature enabled): errorNumbersToAdd: null!.
Can we pass something like Array.Empty<int>() instead null!?

Kind regards,
Aleksandr

[lauxjpn]

Can we pass something like Array.Empty<int>() instead null!?

You can already use either a null value or an empty array in the constructor of MySqlRetryingExecutionStrategy.
Therefore you can also pass either a null value or an empty array to MySqlDbContextOptionsBuilder.EnableRetryOnFailure(int, TimeSpan, [NotNull] ICollection<int>).

I guess the reasoning for the parameter being marked as [NotNull] is, that there are two other overloads that don't take an error number collection at all, so you could always use those, if you don't want to pass a collection:

https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/blob/9be87311c943ffd1389052a05cef4197af991937/src/EFCore.MySql/Infrastructure/MySqlDbContextOptionsBuilder.cs#L53-L75

However, I don't consider this a good reason why the parameter should not be marked as [CanBeNull] anyway.
The SQL Server provider has annotated the parameter correctly as [CanBeNull] but Npgsql uses [NotNull] as we currently do.

We will change the annotation for 5.0 to [CanBeNull].

/cc @roji

[roji]

Looks like this was done in https://github.com/dotnet/efcore/commit/f44520d96bf98d240464ad7585b862526da695fb, will do the same for Npgsql. Thanks for the ping @lauxjpn.