Wrong SQL generated when reusing expressions

[Greg]

Sorry, this will be a bit complex but I found no easy way to reproduce.

Using the Northwind database and OA version 2015.1.225.1

// Basic visitor to replace the lambda parameter
class ParameterTranslator : ExpressionVisitor
{
    public ParameterExpression OldParameter { get; set; }
    public ParameterExpression NewParameter { get; set; }
    protected override Expression VisitParameter(ParameterExpression node)
    {
        return node == this.OldParameter
            ? NewParameter
            : base.VisitParameter(node);
    }
}
static void Main(string[] args)
{
    using( var db = new EntitiesModel1() )
    {
        Expression<Func<Employee, bool>> pred1 = r => r.FirstName == "Andrew";
        var emps = db.Employees.Where(pred1);

        Expression<Func<Employee, bool>> pred2 = r => emps.Any(r2 => r2.EmployeeID == r.ReportsTo);

        // Combine the predicates by replacing pred2's parameter by pred1's and merge the bodies

        var tr = new ParameterTranslator()
        {
            OldParameter = pred2.Parameters[0],
            NewParameter = pred1.Parameters[0],
        };
        var body = Expression.OrElse(pred1.Body, tr.Visit(pred2.Body));
        var pred = Expression.Lambda<Func<Employee, bool>>(body, pred1.Parameters[0]);

        var q = db.Employees.Where(pred);

        var str = q.ToString();
    }
}

The basic idea is I have an arbitrary predicate (in this case employee name="Andrew") from external code that I want to augment in a more complex query (it applies either to the employee or his manager). The important factor seems to be that pred1's body occurs in more than one place in the final expression tree, all occurences are reference-equal. I get the following query:

SELECT *
FROM [Employees] a
WHERE a.[FirstName] = 'Andrew' OR EXISTS
(
 SELECT b.[EmployeeID]
 FROM [Employees] b
 WHERE b.[FirstName] = 'Andrew'
   AND b.[EmployeeID] = b.[ReportsTo]
)

The problem is with the last condition, it references the wrong table. It should be

AND b.[EmployeeID] = a.[ReportsTo]

Is there any way (apart from cloning pred1's body on every use) to work around this? 

 

[Thomas]

Hi Greg,

the LINQ handling in OpenAccess is actively using instance based associations (dictionary<object,x> reference equality), and therefore proper handling or cloning is required in almost all cases.
In your case however, the problem stems from the fact that the emps variable is reused. 
I think you can manipulate your query as follows:

Expression<Func<Employee, bool>> pred1 = r => r.FirstName == "Andrew";
            //var emps = Scope.Extent<Employee>().Where(pred1);

            Expression<Func<Employee, bool>> pred2 = r => Scope.Extent<Employee>().Any(r2 => r2.Id == r.ReportsToID);
            // Combine the predicates by replacing pred2's parameter by pred1's and merge the bodies
            var tr = new ParameterTranslator()
            {
                OldParameter = pred2.Parameters[0],
                NewParameter = pred1.Parameters[0],
            };
            var ex = tr.Visit(pred2.Body);
            var body = Expression.OrElse(pred1.Body, ex);
            var pred = Expression.Lambda<Func<Employee, bool>>(body, pred1.Parameters[0]);

            var q = Scope.Extent<Employee>().Where(pred);

            var str = q.ToString();

This will add the second condition to the first one so that the following SQL is produced:

SELECT  ...a.* ...  
FROM [employee] a WHERE (a.[first_name] = 'Andrew' OR EXISTS (SELECT b.[id]                  FROM [employee] b WHERE b.[id] = a.[ReportsTo])) 

Was that your intent?

Regards,
Thomas
Telerik

 

 

[Greg]

I'm not sure what you mean. The emps variable is used only once. The pred1 expression is what's being reused. In your example the parent employees are not filtered by name.

I have some predicate that filters employees. My intent is to get a list of employees that either satisfy this predicate (this would be the first part of the where clause) or their direct superior satisfies this predicate (this would be the 'exists' part of the where clause).

 

Regards,

Greg

[Thomas]

Hi Greg,

now I understood better.

SELECT ... FROM [employee] a WHERE (a.[first_name] = 'Andrew' OR EXISTS (SELECT 1234567 FROM [employee] b WHERE a.[ReportsTo] = b.[id] AND b.[first_name] = 'Andrew')) 

This I could accomplish by writing
Expression<Func<Employee, bool>> predicate = (r) => r.FirstName == "Andrew" ||  r.ReportsTo.FirstName == "Andrew";

If you want to not duplicate the Andrew-Condition a second time in code but rather generate the expression at runtime, you can follow this approach:

            Expression<Func<Employee, bool>> pred1 = (r) => r.FirstName == "Andrew";
            Expression<Func<Employee, Employee>> pred2 = (r) => r.ReportsTo;
            var pred2body = new ParameterTranslator()
            {
                OldParameter = pred2.Parameters[0],
                NewParameter = pred1.Parameters[0],
            }.Visit(pred2.Body);

            var pred2BodyInPred1 = new ParameterTranslator()
            {
                OldParameter = pred1.Parameters[0],
                NewParameter = pred2.Body
            }.Visit(pred1.Body);

            var body = Expression.OrElse(pred1.Body, pred2BodyInPred1);
            var pred = Expression.Lambda<Func<Employee, bool>>(body, pred1.Parameters);

For that you need to change the property type of NewParameter to Expression in the ParameterTranslator.

Hope that this solves the issue now!

Regards,
Thomas
Telerik

 

 

[Greg]

Hi! 

 

What you suggest is exactly what I originally did, and since the parameter translator pretty much rewrites the entire expression there are no reference duplications and everything seemed to work nice but since this is part of a larger plan I hit the following bug:

http://www.telerik.com/forums/wrong-sql-generated-for-nullable-navigation-properties

If there are other tables involved the generated fragment is no longer an EXISTS expression but a semantically incorrect JOIN.

 

Regards,

Greg

[Thomas]

Hi Greg,

your original code generates a bit more complex expression tree/graph than my last proposal. Can you put that into your bigger plan? I think you could get the queries done with it. The other mentioned issue was not resolved so far.

Regards,
Thomas
Telerik