scottksmith95 / LINQKit

LINQKit is a free set of extensions for LINQ to SQL and Entity Framework power users.
MIT License
1.62k stars 162 forks source link

Nullable properties `NullReferenceException` not thrown. #181

Open elf-is opened 1 year ago

elf-is commented 1 year ago

I encountered a small 'bug' on C# while trying to build a search method using the stringVar.Contains in a predicate. At first It worked fine for every Model I used it on, but if that Model had a null value in some of its properties no error is thrown but the search failed and returned no data. It took me a while to find where the problem was, and that's where I tried checking the values in my Model and found that some were null, usually we'd use the null-conditional operator stringVar?.Contains to check if the variable is null before using a method on it but we can't do that with expression trees (You'd get the following error An expression tree lambda cannot contain conditional access expressions). So my fix was to check if the property is not null and then call the string method.

private static bool Contains(PropertyInfo prop, T x, string searchValue)
    {
        return prop.GetValue(x) != null && prop.GetValue(x).ToString().ToLower().Contains(searchValue);
    }

And then called it in the predicate:

predicate.Or(x => Contains(prop, x, searchValue));

But I was wondering why the error was not thrown and if it was caught why does the predicate fail? P.S: Might be the same problem here #62

sdanyliv commented 1 year ago

If you are building predicate for EF you are doing this in wrong way. PredicateBuilder will not help here. Create question on StackOverflow with detailed description what you ate trying to achieve.

elf-is commented 1 year ago

@sdanyliv I am indeed doing that, I followed the demo you had in the Readme calling AsExpandable() and using predicate.Compile() everything seems to be working fine I don't see what's wrong with that? It's just this single problem that was stopping it from working on some cases because no error was thrown. Here's the link to the demo app I made if you want to test it.

TheConstructor commented 1 year ago

@elf-is You are referring to DataTableService.SearchAColumn? I believe, that you misunderstand the behaviour of EF Core and your Database here, and there isn't a lot LINQKit can do about this. It may also be, that you would need to push for a C# Language specification change.

So first, what does EF Core do with Expressions?

  1. EF Core tries to translate C#-function-calls into SQL
  2. It seems you are using Pomelo.EntityFrameworkCore.MySql as database provider, that documents that stringValue.Contains(value) is translated into expr LIKE pat [ESCAPE 'escape_char'] or LOCATE(substr,str). See https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/wiki/Translations#string-functions
  3. MySQL states for both, that if any argument is NULL, the result is NULL. There is no exception/error here and NULL for most aspects behaves like false in boolean-context

Second, if you intend to use the predicate outside EF Core you basically hit the limits of the C# compiler/C# Language specification. For all I know you can only use the expressions present in .NET 3.5 directly in source code and have it translated for you into an Expression tree, see also https://learn.microsoft.com/en-us/dotnet/csharp/advanced-topics/expression-trees/#limitations . What you can do right now, is to write your own helper like

    public class CoalescingTests
    {

        [Theory]
        [InlineData(null, false)]
        [InlineData("x", true)]
        [InlineData("y", false)]
        public void NullCoalesce(string value, bool expectation)
        {
            Expression<Func<string, bool>> expression = s => s.IfNotNull(str => str.Contains("x"), () => false);
            Assert.Equal(expectation, expression.Invoke(value));

            var expanded = expression.Expand();
            Assert.DoesNotContain("IfNotNull", expanded.ToString());
            Assert.Equal(expectation, expanded.Invoke(value));
        }
    }

    public static class CoalescingExtensions
    {
        [Expandable(nameof(IfNotNullWithDefault))]
        public static TResult IfNotNull<TIn, TResult>(this TIn value, Expression<Func<TIn, TResult>> then, Expression<Func<TResult>> @else)
        {
            return value != null ? then.Invoke(value) : @else.Invoke();
        }

        private static Expression<Func<TIn, Expression<Func<TIn, TResult>>, Expression<Func<TResult>>, TResult>> IfNotNullWithDefault<TIn, TResult>()
        {
            return (value, then, @else) => value != null ? then.Invoke(value) : @else.Invoke();
        }
    }

Calling Expand() results in s => IIF((s != null), s.Contains("x"), False). This differs from ?. in that it does not define a variable for the checked s. If s actually is a computed property, you may want to modify the expansion, to first assign the value to a local variable, as otherwise expanding will spread the property access to all places. Given that you build your predicate it may however be better memory-wise to create the temporary variable once and use it in all predicates.

This would be an expansion with local variable (not supported for .NET 3.5):

        private static Expression<Func<TIn, Expression<Func<TIn, TResult>>, Expression<Func<TResult>>, TResult>> IfNotNullWithDefault<TIn, TResult>()
        {
            var valueParameter = Expression.Parameter(typeof(TIn), "value");
            var value = Expression.Parameter(typeof(TIn), "v");
            var then = Expression.Parameter(typeof(Expression<Func<TIn, TResult>>), "then");
            var @else = Expression.Parameter(typeof(Expression<Func<TResult>>), "else");
            return Expression.Lambda<Func<TIn, Expression<Func<TIn, TResult>>, Expression<Func<TResult>>, TResult>>(
                Expression.Block(
                    new[] {value},
                    Expression.Assign(value, valueParameter),
                    Expression.Condition(
                        Expression.NotEqual(value, Expression.Constant(null, typeof(TIn))),
                        Expression.Invoke(then, value),
                        Expression.Invoke(@else))),
                valueParameter, then, @else
            );
        }

Update: tried to provide shorter code-samples. Defining & assigning variables sadly isn't supported in expression-trees, so it takes up more lines