Nested expression tree when using parameters
Closed this issue · 4 comments
I am currently improving my fork of the library lambda2sql which converts lambdas to SQL clauses using your library and have an issue when using parameters in the .and()
or .or()
methods in Predicate lambdas.
Here's some sample code:
SqlPredicate<IPerson> personPredicate = p -> p.getName() == "Donald";
personPredicate = personPredicate.and(p -> p.getName() == "Steve");
works fine and returns the desired output of name = 'Donald' AND name = 'Steve'
.
But when I run this code:
var name1 = "Donald";
var name2 = "Steve";
SqlPredicate<IPerson> personPredicate = p -> p.getName() == name1;
personPredicate = personPredicate.and(p -> p.getName() == name2);
I get the output name = 'Donald' AND name = 'Donald'
. Somehow the expression tree becomes nested with the second predicate and thus the ParameterExpression does not deliver the correct index value when going through the .visit(ParameterExpression)
implementation. The index is 0 for the second Predicate, so it just grabs the first parameter again.
Every lambda is evaluated in its own context, exactly like in a Java program. Consider this expression: p -> p.getName() == name2
: in its context there are 2 captured parameters name2
and p
. What would be the order? Decided by Java compiler. And of course it also correctly generates the invocation code passing arguments in the right order. JaQue parses all this, generating one expression tree.
How one can correctly resolve parameters in a context of particular Invocable (Lambda)? By looking into arguments:
You can see that 1st argument to the LambdaExpression is a ConstantExpression "Steve". Note, that theoretically some value may be passed as a parameter several times until used, exactly as it's happening with the 2nd argument - it's actually a ParameterExpression from the outer scope. SimpleExpressionVisitor
has 2 protected methods: visitArguments
and visitParameters
to help the visitors handle parameter/arguments shifts.
- it must be also noted that MS Expression Tree implementation behaves in the same way in case of compositions.
Okay, thanks for the tip! So the SimpleExpressionVisitor
implements ExpressionVisitor<Expression>
. How can I now generate my SQL from that, since I can't use my StringBuilder
anymore?
I made a look on your code. Moving to SimpleExpressionVisitor
is not strictly necessary, in the master version I see that you only add to parameters collection (should be called arguments) and never clean. So if you have nested lambdas you will have a mess. Some kind of stack semantics should be used.
Yet, I would consider refactoring and using SimpleExpressionVisitor
. Its main benefit is that it does full tree traversal for you. So you don't have to worry, you forgot something (e.g. you may remove visit(LambdaExpression<?> e)
override). Also it does not make much sense to have a StringBuilder
as a T
for ExpressionVisitor
, since I don't see a reason why it would ever be not the same instance. So you can simply store it as a field. (The reason SimpleExpressionVisitor
has Expression
as T
is to support tree rewriting scenarios).
In any case you will need to implement some sort of stack for arguments.
Thank you for your help, it works now!