6

I get an error when trying to combine two expressions with AndAlso.

Step 1: building expression for EF object with expression tree:

public override Expression<Func<T, bool>> ToExpression()
{
    var expressionParameter = Expression.Parameter(typeof(T), "p");
    var expressionField = Expression.PropertyOrField(expressionParameter, field);
    var expressionConstraint = Expression.Constant(value);

    BinaryExpression expression = Expression.Equal(expressionField, expressionConstraint);

    return Expression.Lambda<Func<T, bool>>(expression, expressionParameter);
}

If I'm calling .ToExpression(); and running this for one expression, this code works fine with EF, it produces expression:

{ p => (p.MyField1 == "X") }

But when I'm trying to do

Step 2: combine two expressions with AndAlso:

public override Expression<Func<T, bool>> ToExpression()
{
    Expression<Func<T, bool>> leftExpression = left.ToExpression();
    Expression<Func<T, bool>> rightExpression = right.ToExpression();

    BinaryExpression andExpression = Expression.AndAlso(leftExpression.Body, rightExpression.Body);

    return Expression.Lambda<Func<T, bool>>(andExpression, leftExpression.Parameters.Single());
}

This produces an expression:

{ p => ((p.MyField1 == "X") AndAlso (p.MyField2 == "Y")) }

It seems fine to me, but when trying to call it with .Where(expression) I get this error:

System.InvalidOperationException: The LINQ expression 'p' could not be translated. Either rewrite the query in a form that can be translated, or switch to client evaluation explicitly by inserting a call to 'AsEnumerable', 'AsAsyncEnumerable', 'ToList', or 'ToListAsync'

Any idea why separate expressions work but combined with AndAlso don't?

Robert Harvey
  • 178,213
  • 47
  • 333
  • 501
Carlos28
  • 2,381
  • 3
  • 21
  • 36
  • Have you considered using PredicateBuilder / LinqKit instead of doing this manually by yourself? – Dai Jul 17 '23 at 13:49
  • No, I will take a look, but still curious why this doesn't work – Carlos28 Jul 17 '23 at 13:55
  • I imagine it fails because `AndAlso` is the VB.NET short-circuiting version of VB.NET's `And` combinator (which famously is _not_ short-circuiting) - whereas SQL (which is what I assume you're using this for?) does not define sequence-order for expressions so it makes no sense to support `AndAlso` instead of just `And`. – Dai Jul 17 '23 at 13:56
  • I have tried to also use `And` instead `AndAlso` but im getting same result for expression: `{p => ((p.MyField1 == "X") And (p.MyField2 == "Y"))}`, and yes this is for SQL – Carlos28 Jul 17 '23 at 14:01
  • I'd like to play around with this - where do I put the `ToExpression` method? – Dai Jul 17 '23 at 14:07
  • 1
    When you combine the expressions, I suspect ef core is not able to take "p" as same parameter reference , rather consider them different, perhaps you could use expressionVisitor to replace parameter https://stackoverflow.com/a/41652669/1398461 – kuldeep Jul 17 '23 at 14:13
  • @Dai I have a generic class where T is my EF object, just passing a `field` and `value` for filtering and the result of 'ToExpression()' is passed to `.Where()` in EF call, can provide more complete code if needed – Carlos28 Jul 17 '23 at 14:22
  • 1
    Lambda expressions cannot be combined this way because they use different `ParameterExpression` instances (the name doesn't matter). `Expression.Invoke` is limited and not always supported, you need `ParameterReplacer`, there are a lot of examples here. – Ivan Stoev Jul 17 '23 at 17:45
  • @Carlos28 what's the point of this? SQL has no `AndAlso` because the order of the operands doesn't matter. The *best* SQL this `AndAlso` can produce is just `AND`. The database will use the actual operand, table schema, indexes *and* data statistics to come up with an execution plan that works on streams of rows. Short-circuiting has no meaning , unless the parser determines a condition is always true and eliminates it, like `AND 1=1` – Panagiotis Kanavos Jul 20 '23 at 08:09

2 Answers2

1

Your Expression.Lambda calls probably need to start with a call to Expression.Invoke like this:

var expr = Expression.Invoke(expression, expressionParameter);
return Expression.Lambda<Func<T, bool>>(expr, expressionParameter);
Bron Davies
  • 5,930
  • 3
  • 30
  • 41
0

Implementing ParameterReplacer did the trick, implementation that I have found and used:

    internal class ReplaceExpressionVisitor : ExpressionVisitor
{
    private readonly Expression _oldValue;
    private readonly Expression _newValue;

    public ReplaceExpressionVisitor(Expression oldValue, Expression newValue)
    {
        _oldValue = oldValue;
        _newValue = newValue;
    }

    public override Expression Visit(Expression node) => node == _oldValue ? _newValue : base.Visit(node);
}

Thank's for your suggestions.

Carlos28
  • 2,381
  • 3
  • 21
  • 36
  • What SQL did this emit? Was it identical to `AND` perhaps? SQL has no `AndAlso` and in fact, the order doesn't matter when it comes to generating an execution plan. The execution plan will pick indexes and filtering strategies that work on streams of data. – Panagiotis Kanavos Jul 20 '23 at 08:06
  • @PanagiotisKanavos, EF Core translates `Expression.AndAlso` (and only this binary expression) into logical AND. `Expression.And` is bitwise operator and has different meaning. – Svyatoslav Danyliv Jul 21 '23 at 11:54