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

Fix EF Core issue when using MySqlGeometry.Value #829

Merged
merged 1 commit into from
Jun 8, 2020

Conversation

lauxjpn
Copy link
Contributor

@lauxjpn lauxjpn commented Jun 8, 2020

EF Core throws an exception, when using MySqlGeometry, because there is some very general code in Microsoft.EntityFrameworkCore.Storage.Internal.DbParameterCollectionExtensions.FormatParameterValue(StringBuilder builder, object parameterValue), that is unable to handle the ReadOnlySpan<byte> type of MySqlGeometry.Value.

This PR changes the type to byte[] instead. Technically, this is a breaking change for anyone who used the Value property before. But because of the implicit cast between byte[] and ReadOnlySpan<byte>, this should be less of an issue.

In any case, if there is an issue in someones code, this change will lead to a compiler error, that can be easily fixed by replacing Value with either Value.AsSpan() or new ReadOnlySpan<byte>(Value).

…byte>` will throw an exception in `Microsoft.EntityFrameworkCore.Storage.Internal.DbParameterCollectionExtensions.FormatParameterValue(StringBuilder builder, object parameterValue)`.

Signed-off-by: Laurents Meyer <laucomm@gmail.com>
@bgrainger
Copy link
Member

Technically, this is a breaking change for anyone who used the Value property before.

I don't think MySqlGeometry gets a lot of use (see this comment: #70 (comment)), so I'm not too worried about a breaking change.

This change does make the MySqlGeometry API more compatible with Connector/NET (which does return a byte[]) but I'm not happy with returning a mutable byte[] that directly exposes the private data; this was why ReadOnlySpan<byte> was used in the first place.

And shouldn't you submit a PR to fix EFCore, adding ReadOnlySpan<byte> support in https://github.com/dotnet/efcore/blob/252dee5c84eb5d7d1ef2254974f1100f3cb9cc1a/src/EFCore.Relational/Storage/Internal/DbParameterCollectionExtensions.cs#L142-L192? 😀

@bgrainger bgrainger merged commit 4806b9e into mysql-net:master Jun 8, 2020
@bgrainger
Copy link
Member

I'm not happy with returning a mutable byte[] that directly exposes the private data

Fixed in afa4bf5.

@bgrainger
Copy link
Member

Fixed in 0.69.0.

@lauxjpn
Copy link
Contributor Author

lauxjpn commented Jun 9, 2020

I'm not happy with returning a mutable byte[] that directly exposes the private data; this was why ReadOnlySpan was used in the first place.

Good point!

Fixed in 0.69.0.

Thanks, that was quick!

And shouldn't you submit a PR to fix EFCore, adding ReadOnlySpan support in https://github.com/dotnet/efcore/blob/252dee5c84eb5d7d1ef2254974f1100f3cb9cc1a/src/EFCore.Relational/Storage/Internal/DbParameterCollectionExtensions.cs#L142-L192?

I am thinking about it. Might be worth it for EF Core 5.

@lauxjpn lauxjpn deleted the fix/efcore_issue branch June 9, 2020 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants