Pomelo.EntityFrameworkCore.MySql
Pomelo.EntityFrameworkCore.MySql copied to clipboard
Change `Match` extension methods and introduce `IsMatch` ones
The Match() methods should return the relevance value of the match in the same way as the MATCH...AGAINST clause does in MySQL.
For convenience, we keep the previous functionality to just return true or false around, but rename those methods to IsMatch(), that just translate to MATCH() ... AGAINST() > 0.0.
Fixes #1679
Did you intentionally skip the arguments[3] is SqlParameterExpression parameter case for the new .Match-mapper?
Did you intentionally skip the
arguments[3] is SqlParameterExpression parametercase for the new.Match-mapper?
~~@TheConstructor I'm not sure which line you are referring to. Just directly comment on the line (using the Files changed tab).~~
Ah, I see now what you mean. Yes, that is intentional. I don't see a simple way for the generated SQL to switch between the 3 available MySqlMatchSearchMode options, ensure the usage of available fulltext indices and make it work for MySQL and MariaDB.
That's why for Match():
/// <param name="searchMode">The mode to performed the search with. Needs to be a constant value or throws otherwise.</param>
I guess you could try a CASE WHEN expression, and keep recommending to use a constant value.
Just re-read the comment. I guess using index would be interesting, if used in ORDER BY, and Maria DB-support would also be great, but implementing CASE WHEN shouldn't be worse than no translation.
Just re-read the comment. I guess using index would be interesting, if used in ORDER BY, and Maria DB-support would also be great, but implementing
CASE WHENshouldn't be worse than no translation.
The chosen path that EF Core takes (which I think is reasonable), is to provide only performant options by default, because most users will not investigate performance issues and just expect things to work well out-of-the-box.
That provides us with the following options:
- Only support a constant value and throw otherwise (current PR).
- Make a partially unoptimized implementation (like using
CASE) opt-in. - Support only optimized/index using
CASEimplementations and throw otherwise (i.e. for MariaDB). - Revert to using explicit methods (like
MatchBooleanMode()) that kind of force users to explicitly choose a mode.
Which I see as follows:
- is simple, but with a higher chance to throw at runtime.
- seems too much work to support a specific corner case.
- is reasonable, but might be confusing when switching databases.
- is what other providers do: more redundant but kind of forces the users hand.
So in my opinion, all options except 2 are probably fine, all having their individual pros and cons.
@TheConstructor Do you think we should implement option 3 instead of option 1?
I dislike, that the exceptions are only occuring at runtime, as it is not always trivial to test all possible query variants. The user is already able to implement CASE WHEN, so maybe an explaining exception would be the best approach. 4 would make upgrading more difficult, but would remove this problem completely.
So after your explanation I would probably add a specific exception, that states that the last argument needs to be a constant for now.
Another question, that came to my mind, is, if we should give the user a way to pass a value other than 0.0 to IsMatch.
@TheConstructor Sorry, I forget this one is still open! Since we have already published a silver release, this PR will unfortunately not make it into 7.0.x anymore. :disappointed:
@lauxjpn so when to expect some movement now? 8.0 sounds like autumn?
@TheConstructor This should make it into 8.0.0-beta.2.
Another question, that came to my mind, is, if we should give the user a way to pass a value other than 0.0 to IsMatch.
Since Match() does still exist (in addition to IsMatch()), I think it is simple enough to just write Match(...) > 0.42 for anyone who wants to customize the behavior.