Support for translating Regex.Replace and Regex.Count

[WhatzGames]

Recently I came across a case where the regex_replace function would have been helpful.
Also the regex_count function could have come in handy at that time.

Though it was easy to workaround the missing features, I thought I'd create an issue here.
I have been tinkering on my own fork already, so if there's interest for it I will gladly create a PR for it.

[roji]

Putting in the backlog.

You're certainly welcome to submit a PR - though it may take me a while to get around to reviewing it. If you want to take a stab at it, take a look at the existing support for regex searching in the provider to see how it's done overall.

[WhatzGames]

After getting your feedback for #2936 I looked over my PR for this one.
I noticed during some more tinkering in SQL, that regex_count() also returns NULL when NULL ist provided for any of it's arguments.
This means, that there is a discrepancy between the sql return value and the .NET Regex.Count().
regex_count() being int? and Regex.Count() being plain int.

would you say in such a case, that Regex.Count() should not be used and a custom RegexCount(this DbFunctions _, ...) extension instead?
Or is there another preferred viable approach you have in mind?

[roji]

@WhatzGames sorry for taking some time to get back to you.

Yeah, the question of nullability is a tricky one, since .NET and SQL behave very differently around nulls. In many cases, a .NET function throws an exception when given a null argument, where in SQL that causes the corresponding function to return null instead (aka "null propagation" behavior). EF function translations are unfortunately not always fully consistent around how they deal with nulls.

As a general rule, it seems more useful to approximate regular .NET method signatures for EF.Functions we introduce. That is, if a parameter really is optional (and and omitting it by passing in a null) provides a useful return value (which usually would mean it isn't null), then it makes sense for that parameter to be nullable in .NET. However, if the parameter would only be nullable for SQL null propagation, and conceptually you'd expect the .NET function to throw instead, then it's better to not have nullable parameters, since that makes for a worse .NET experience. If we have nullable return values, that forces the user to check for null (in C#) every time they call this function, even when they pass non-null inputs.

If you look at the EF.Functions for SQL Server, you can see that most parameters and return values are non-nullable. Specifically for the DateDiff* functions, there are overloads both for nullable and non-nullable; this is where EF isn't fully consistent, and note that this is only possible with value type parameters (it's not possible to overload on nullability with .NET reference types).

To summarize... If all the above makes sense, I think we should probably go back to #2936 and amend ToDate/ToTimestamp to have non-nullable parameters - is that something you can do please? Similarly, regex_count() accepts and returns NULL as part of the standard SQL NULL propagation behavior - this is a case where in .NET we'd actually accept an exception. As such, I'd expect the EF.Functions method to also return a non-nullable value.

[WhatzGames]

@roji no worries, busy times take their toll.

First of all, thanks for the helpful explanation.

Considering your feedback regarding regex_count, I take it that using Regex.Count() is a suitable option then.
The .NET method itself throws when given a null value.
Translating this should have the same effect in the end.
Therefore no extra EF.Function is needed.

[roji]

@WhatzGames yep, that makes sense - and it's always better to just translate a built-in function when possible (I'd definitely not expect us to introduce EF.Functions for such a nullability difference).

[WhatzGames]

@roji I hope it's okay, that I ping you.
Just wanted to let you know that the PR is ready for review.

[roji]

Thanks @WhatzGames - unfortunately I'm very busy in this period, it may take me a while before I can review your PR... I'll do my best to do this for 9.0 though.