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

Make StringComparison translations opt-in #1390

Closed
lauxjpn opened this issue Apr 22, 2021 · 1 comment · Fixed by #1391
Closed

Make StringComparison translations opt-in #1390

lauxjpn opened this issue Apr 22, 2021 · 1 comment · Fixed by #1391

Comments

@lauxjpn
Copy link
Collaborator

lauxjpn commented Apr 22, 2021

After the discussion of #996 (comment) (and previous comments) with @roji, that continued by email, we came to the conclusion that it would be best to throw as a default, when a StringComparison method is about to be translated, unless users have explicitly opted in to the feature.

The background is, that depending on the collations used, the scenario and the involved indices, MySQL might not be able to use indices at all and thus performance could suffer.

We will assume, that when users are explicitly opting in to the feature in the future, that they are aware of the possible consequences.

@lauxjpn
Copy link
Collaborator Author

lauxjpn commented May 12, 2021

@kanaday We marked it as a breaking-change, as we decided in the quite long discussion, that conformity to other providers and performance by default was more important than keeping compatibility.

However, we made the previous functionality opt-in for convenience, so that anybody who depends on the previous behavior, and doesn't want to use the new EF.Functions.Collation() syntax, can explicitly enable it again in an instant, and thereby confirming to be aware of possible performance implications.

To throw using these methods without opting in was a deliberate choice. It is the behavior that EF Core provides by default, but more importantly, if we would have just ignored the StringComparison parameter (and logged a warning), it could be quite hard for users to track the issue down, as many don't have warnings enabled in production and implicitly ignoring casings is difficult to trace.

So throwing ensures, that this otherwise subtle change does not go unnoticed and does not lead to data corruption.

As upgrading from 3.x to 5.0 is a major release change, and we try to keep compliance with SerVer2, such an upgrade is likely to contain breaking changes. So make sure when upgrading, that you check against them and test your application, before deploying it to production.

We label important breaking-changes, that might impact an application at runtime (instead of at compile time), accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment