Wrong SQL generated when reusing expressions

6 posts, 0 answers
  1. Greg
    Greg avatar
    44 posts
    Member since:
    Jun 2012

    Posted 07 Apr 2015 Link to this post

    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? 

     

  2. Thomas
    Admin
    Thomas avatar
    590 posts

    Posted 08 Apr 2015 Link to this post

    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
     
    OpenAccess ORM is now Telerik Data Access. For more information on the new names, please, check out the Telerik Product Map.
     
  3. DevCraft banner
  4. Greg
    Greg avatar
    44 posts
    Member since:
    Jun 2012

    Posted 08 Apr 2015 in reply to Thomas Link to this post

    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

  5. Thomas
    Admin
    Thomas avatar
    590 posts

    Posted 10 Apr 2015 Link to this post

    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
     
    OpenAccess ORM is now Telerik Data Access. For more information on the new names, please, check out the Telerik Product Map.
     
  6. Greg
    Greg avatar
    44 posts
    Member since:
    Jun 2012

    Posted 10 Apr 2015 in reply to Thomas Link to this post

    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

  7. Thomas
    Admin
    Thomas avatar
    590 posts

    Posted 13 Apr 2015 Link to this post

    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
     
    OpenAccess ORM is now Telerik Data Access. For more information on the new names, please, check out the Telerik Product Map.
     
Back to Top
DevCraft banner