Dynamically built LINQ query no longer working in NH3.0

[j gwood]

After upgrading from NH2.1.2GA to NH3.0GA, several of our LINQ queries
that use the PredicateBuilder class (http://www.albahari.com/nutshell/
predicatebuilder.aspx) no longer work. The PredicateBuilder class is
used to aid in the construction of dynamically created Expressions
where an Expression must be initialized before looping through code to
add values to the Expression. The LINQ queries result in the
following error (full stack trace at the end of the post):

NHibernate.Hql.Ast.ANTLR.InvalidPathException : Invalid path:
'item.SerialNumber' [.Where(NHibernate.Linq.NhQueryable`1[NHibernate.Test.NHSpecificTest.NHPred.Animal],
Quote((f, ) => (AndAlso(p1, (item, ) =>
(String.op_Equality(item.SerialNumber, p2))f))), )]

It seems as if the Expressions aren't being individually evaluated.
NH assigns the first variable, 'f', to the 'Animal' entity, but then
cannot associate the 'item' variable to the same entity even though
the 'item' variable does represent the 'Animal' entity. When viewing
the Expression Tree, the ParameterExpression elements are as follows:

ParameterExpression
- Name: String: "item"
- NodeType: ExpressionType: "Parameter"
- Type: Type: "Animal"

ParameterExpression
- Name: String: "f"
- NodeType: ExpressionType: "Parameter"
- Type: Type: "Animal"

Is this a bug in NHibernate or a user error?

I am using the Animal class and mapping files found in the NHibernate
unit test directories. My unit test is as follows:

[Test]
public void
NH30LinqQueryWithExpressionsDifferentInputParameterValueTest()
{
//Simulate data coming from web form.
string searchOnName = string.Empty;
const string searchOnSerialNumber = "444";

using (ISession session = this.OpenSession())
{
//Since we will be 'and-ing' our conditions, we can just start with
a 'True' value to initialize
// the Expression and add all our conditions to it.
Expression<Func<Animal, bool>> criteria =
PredicateBuilder.True<Animal>(); // return f => true;

if (!string.IsNullOrEmpty(searchOnName))
//NOTE: The .And() method is an extension method provided by
PredicateBuilder.
criteria = criteria.And(item => item.Description == searchOnName);

if (!string.IsNullOrEmpty(searchOnSerialNumber))
//NOTE: The .And() method is an extension method provided by
PredicateBuilder.
criteria = criteria.And(item => item.SerialNumber ==
searchOnSerialNumber);

//Run the search.
if (null != criteria)
{
var animals = session.Query<Animal>().Where(criteria).ToList();
Assert.AreEqual(1, animals.Count);
}
else
Assert.Fail("Criteria variable should not be null.");
}
}

Full stack trace:

NHibernate.Hql.Ast.ANTLR.InvalidPathException : Invalid path:
'item.SerialNumber' [.Where(NHibernate.Linq.NhQueryable`1[NHibernate.Test.NHSpecificTest.NHPred.Animal],
Quote((f, ) => (AndAlso(p1, (item, ) =>
(String.op_Equality(item.SerialNumber, p2))f))), )]
at
NHibernate.Hql.Ast.ANTLR.Util.LiteralProcessor.LookupConstant(DotNode
node)
at NHibernate.Hql.Ast.ANTLR.Tree.DotNode.Resolve(Boolean generateJoin,
Boolean implicitJoin, String classAlias, IASTNode parent)
at NHibernate.Hql.Ast.ANTLR.Tree.FromReferenceNode.Resolve(Boolean
generateJoin, Boolean implicitJoin)
at NHibernate.Hql.Ast.ANTLR.HqlSqlWalker.Resolve(IASTNode node)
at NHibernate.Hql.Ast.ANTLR.HqlSqlWalker.expr()
at NHibernate.Hql.Ast.ANTLR.HqlSqlWalker.exprOrSubquery()
at NHibernate.Hql.Ast.ANTLR.HqlSqlWalker.comparisonExpr()
at NHibernate.Hql.Ast.ANTLR.HqlSqlWalker.logicalExpr()
at NHibernate.Hql.Ast.ANTLR.HqlSqlWalker.logicalExpr()
at NHibernate.Hql.Ast.ANTLR.HqlSqlWalker.whereClause()
at NHibernate.Hql.Ast.ANTLR.HqlSqlWalker.unionedQuery()
at NHibernate.Hql.Ast.ANTLR.HqlSqlWalker.query()
at NHibernate.Hql.Ast.ANTLR.HqlSqlWalker.selectStatement()
at NHibernate.Hql.Ast.ANTLR.HqlSqlWalker.statement()
at NHibernate.Hql.Ast.ANTLR.HqlSqlTranslator.Translate()
at NHibernate.Hql.Ast.ANTLR.QueryTranslatorImpl.Analyze(String
collectionRole)
at
NHibernate.Hql.Ast.ANTLR.QueryTranslatorImpl.DoCompile(IDictionary`2
replacements, Boolean shallow, String collectionRole)
at
NHibernate.Hql.Ast.ANTLR.ASTQueryTranslatorFactory.CreateQueryTranslators(IASTNode
ast, String queryIdentifier, String collectionRole, Boolean shallow,
IDictionary`2 filters, ISessionFactoryImplementor factory)
at
NHibernate.Hql.Ast.ANTLR.ASTQueryTranslatorFactory.CreateQueryTranslators(String
queryIdentifier, IQueryExpression queryExpression, String
collectionRole, Boolean shallow, IDictionary`2 filters,
ISessionFactoryImplementor factory)
at NHibernate.Engine.Query.HQLExpressionQueryPlan..ctor(String
expressionStr, IQueryExpression queryExpression, String
collectionRole, Boolean shallow, IDictionary`2 enabledFilters,
ISessionFactoryImplementor factory)
at NHibernate.Engine.Query.HQLExpressionQueryPlan..ctor(String
expressionStr, IQueryExpression queryExpression, Boolean shallow,
IDictionary`2 enabledFilters, ISessionFactoryImplementor factory)
at
NHibernate.Engine.Query.QueryPlanCache.GetHQLQueryPlan(IQueryExpression
queryExpression, Boolean shallow, IDictionary`2 enabledFilters)
at
NHibernate.Impl.AbstractSessionImpl.GetHQLQueryPlan(IQueryExpression
queryExpression, Boolean shallow)
at NHibernate.Impl.AbstractSessionImpl.CreateQuery(IQueryExpression
queryExpression)
at NHibernate.Linq.NhQueryProvider.PrepareQuery(Expression expression,
ref IQuery query, ref NhLinqExpression nhQuery)
at NHibernate.Linq.NhQueryProvider.Execute(Expression expression)
at NHibernate.Linq.NhQueryProvider.Execute(Expression expression)
at Remotion.Data.Linq.QueryableBase`1.GetEnumerator()
at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
at System.Linq.Enumerable.ToList(IEnumerable`1 source)
at
NHibernate.Test.NHSpecificTest.NHPred.LinqTest.NH30LinqQueryWithExpressionsDifferentInputParameterValueTest()
in LinqTest.cs: line 203

[Fabian Schmied]

Hi,

 

PredicateBuilder uses a neat little trick that is, however, not supported by NHibernate yet: to fit a LambdaExpression in a place where no LambdaExpression is allowed, it builds an InvocationExpression around the LambdaExpression. NHibernate does not evaluate that InvocationExpression.

 

I've opened a JIRA issue to solve this at the re-linq level because this is something all LINQ providers based on re-linq (not only NHibernate) would benefit from: https://www.re-motion.org/jira/browse/RM-3656.

I can't promise anything, but I'm planning to implement a few new features for re-linq this January, so this might be working soon in re-linq - and, after they take over the latest re-linq version, in NHibernate as well.

 

In the meantime, you could use a modified version of PredicateBuilder (modified from http://www.albahari.com/nutshell/predicatebuilder.aspx):

 

public static Expression<Func<T, bool>> And<T> (this Expression<Func<T, bool>> expr1,
                                                       Expression<Func<T, bool>> expr2)
{

  var adaptedExpr2Body = ReplacingExpressionTreeVisitor.Replace (expr2.Parameters.Single(), expr1.Parameters.Single(), expr2.Body);
  return Expression.Lambda<Func<T, bool>>
        (Expression.AndAlso (expr1.Body, adaptedExpr2Body), expr1.Parameters);
}

 

(Untested, from my head. ReplacingExpressionTreeVisitor is a part of re-linq and resides in the Remotion.Data.Linq.Parsing,ExpressionTreeVisitors namespace.)

"Or" would need to be adapted in a similar fashion.

 

Hope this helps,

Fabian

[José F. Romaniello]

You are wrong, and you should avoid Expression.Invoke, my linq spec project does exactly this, and it works.

http://linqspecs.codeplex.com/

given expr1 and expr2, you can always do this:

ParameterExpression param = expr1.Parameters[0];
if (ReferenceEquals(param, expr2.Parameters[0]))
{
	return Expression.Lambda<Func<T, bool>>(Expression.AndAlso(expr1.Body, expr2.Body), param);
}
return Expression.Lambda<Func<T, bool>>(Expression.AndAlso(expr1.Body, Expression.Invoke(expr2, param)), param);

See my AndSpecification here:

http://linqspecs.codeplex.com/SourceControl/changeset/view/f8b332648229#LinqSpecs%2fAndSpecification.cs

2011/1/7 Fabian Schmied <fabian....@gmail.com>

--
You received this message because you are subscribed to the Google Groups "nhusers" group.
To post to this group, send email to nhu...@googlegroups.com.
To unsubscribe from this group, send email to nhusers+u...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/nhusers?hl=en.

[José F. Romaniello]

sorry, i was wrong this time... let me help to clarify what i did to get it working on linqspecs:

Previously i had this code, and it doesn't work with nhibernate 3 linq provider:

ParameterExpression·param·=·Expression.Parameter(typeof(T),·"x");

Expression.Lambda<Func<T,·bool>>(

Expression.AndAlso(

Expression.Invoke(expr1,·param),

Expression.Invoke(expr2,·param)),·param);

So, with the help of Fabio we changed to this:

ParameterExpression param = expr1.Parameters[0];
if

 (ReferenceEquals(param, expr2.Parameters[0]))
{
	// simple version

	return Expression.Lambda<Func<T, bool>>(Expression.AndAlso(expr1.Body, expr2.Body), param);
}

// otherwise, keep expr1 "as is" and invoke expr2

return Expression.Lambda<Func<T, bool>>(Expression.AndAlso(expr1.Body, Expression.Invoke(expr2, param)), param);

This do works with nh3 linq provider.

I don't get the difference yet

2011/1/7 José F. Romaniello <jfroma...@gmail.com>

[Fabian Schmied]

> You are wrong, [...]

The change I proposed (in re-linq) would make Invoke just work in all
re-linq-based providers.
The workaround using the ReplacingExpressionTreeVisitor completely
avoids InvocationExpressions, and I strongly believe it will work
right now, with the existing NHibernate 3.0.

Where exactly am I wrong?

> ParameterExpression param = expr1.Parameters[0];
> if (ReferenceEquals(param, expr2.Parameters[0]))
> {
> return Expression.Lambda<Func<T, bool>>(Expression.AndAlso(expr1.Body,
> expr2.Body), param);
> }
> return Expression.Lambda<Func<T, bool>>(Expression.AndAlso(expr1.Body,
> Expression.Invoke(expr2, param)), param);

I'm not sure if the workaround you've posted will always work: you're
still using Expression.Invoke in those cases where the parameters are
not the same for both expressions. I believe that those cases would
still cause the exception described by the original poster.

Fabian

[José F. Romaniello]

I think that for an strange reason, Expr.Invoike in one side doesn't break. I've tests on that here:

http://linqspecs.codeplex.com/SourceControl/changeset/view/f8b332648229#LinqSpecs.IntegrationalTests%2fAndQueriesFixture.cs

http://linqspecs.codeplex.com/SourceControl/changeset/view/f8b332648229#LinqSpecs.IntegrationalTests%2fSampleQueries%2fByAlbumTitle.cs

But yes, i think you are right, ant it should works calling invoke on both sides.

2011/1/7 Fabian Schmied <fabian....@gmail.com>

[James Crowley]

In case anyone else comes across this and are looking for a short-term fix, I just ended up just chaining several "Where" expressions together instead - so

var item = something.Where(t=>t.xx)

if (condition)

   item = item.Where(t=>t.yy)

and so on.

[Variant]

There is a modified version of the PredicateBuilder that works fine
with NH3. It has a little override to the ExpressionBuilder that
renames the lambda parameter and rebuilds the expression tree.

It can be found there:
http://www.primordialcode.com/Blog/Post/nhibernate-linq-dynamic-filtering-lambda-expressions

I did encouter a problem with that solution when nested lambdas are in
place and there is no need to rename the parameter of the nested
lambda.
I changed the code slightly to overcome this scenario:

protected override Expression VisitParameter(ParameterExpression p)
{
if (p.Type == _newParam.Type)
return _newParam;
else
return p;

}



On Jan 10, 5:07 pm, James Crowley <james.crow...@gmail.com> wrote:
> In case anyone else comes across this and are looking for a short-term fix,
> I just ended up just chaining several "Where" expressions together instead
> - so
>
> var item = something.Where(t=>t.xx)
> if (condition)
>    item = item.Where(t=>t.yy)
>
> and so on.
>

> On 7 January 2011 11:46, José F. Romaniello <jfromanie...@gmail.com> wrote:
>
>
>
>
>
>
>
> > I think that for an strange reason, Expr.Invoike in one side doesn't break.
> > I've tests on that here:
>

> >http://linqspecs.codeplex.com/SourceControl/changeset/view/f8b3326482...
> > <http://linqspecs.codeplex.com/SourceControl/changeset/view/f8b3326482...>
> >http://linqspecs.codeplex.com/SourceControl/changeset/view/f8b3326482...
>
> > <http://linqspecs.codeplex.com/SourceControl/changeset/view/f8b3326482...>But

> > yes, i think you are right, ant it should works calling invoke on both
> > sides.
>

> > 2011/1/7 Fabian Schmied <fabian.schm...@gmail.com>

>
> >> I'm not sure if the workaround you've posted will always work: you're
>
> >> still using Expression.Invoke in those cases where the parameters are
> >> not the same for both expressions. I believe that those cases would
> >> still cause the exception described by the original poster
>
> >  --
> > You received this message because you are subscribed to the Google Groups
> > "nhusers" group.
> > To post to this group, send email to nhu...@googlegroups.com.
> > To unsubscribe from this group, send email to

> > nhusers+u...@googlegroups.com<nhusers%2Bunsu...@googlegroups.com >
> > .

[Fabian Schmied]

Hi,

> There is a modified version of the PredicateBuilder that works fine
> with NH3. It has a little override to the ExpressionBuilder that
> renames the lambda parameter and rebuilds the expression tree.
>
> It can be found there:
> http://www.primordialcode.com/Blog/Post/nhibernate-linq-dynamic-filtering-lambda-expressions

Great! Just a few comments:

- Since NHibernate uses re-linq, it already contains an Expression
Visitor base class (ExpressionTreeVisitor) - no need to implement your
own.

- As I've posted before, re-linq also contains a
ReplacingExpressionTreeVisitor class that can be used to replace the
parameters. No need to define your own ParameterModifier.

- In your RebuildExpressionIfNeeded method, you check if "p.Name !=
expr2.Parameters[0].Name". Maybe NHibernate has special code to also
compare parameter names, but usually, parameter expressions are
compared by reference, not by name, so you'd need to change the check
to: "p != expr2.Parameters[0]". You can omit rebuilding the expression
tree only if the references match. In fact, I just tried with Linq to
Objects, and your code doesn't work there unless you change the check
in RebuildExpressionIfNeeded.

- Your ParameterModifier will yield invalid results if there is any
other ParameterExpression involved in the expression tree besides the
one you want to replace (eg, in the nested LambdaExpression case you
hint about below). To overcome this, change it so that it compares the
current expression in VisitParameter with the original parameter
expression (by reference), and perform the replacement only if they
match.

For completeness, here's again my own variation of And and Or using
re-linq's ReplacingExpressionTreeVisitor:

public static Expression<Func<T, bool>> And<T> (
this Expression<Func<T, bool>> expr1,
Expression<Func<T, bool>> expr2)
{
var adaptedExpr2Body = ReplacingExpressionTreeVisitor.Replace (

expr2.Parameters[0],
expr1.Parameters[0],

expr2.Body);
return Expression.Lambda<Func<T, bool>> (
Expression.AndAlso (expr1.Body, adaptedExpr2Body),
expr1.Parameters);
}

public static Expression<Func<T, bool>> Or<T> (

this Expression<Func<T, bool>> expr1,
Expression<Func<T, bool>> expr2)
{
var adaptedExpr2Body = ReplacingExpressionTreeVisitor.Replace (

expr2.Parameters[0],
expr1.Parameters[0],

expr2.Body);
return Expression.Lambda<Func<T, bool>> (
Expression.AndAlso (expr1.Body, adaptedExpr2Body),
expr1.Parameters);
}

> I did encouter a problem with that solution when nested lambdas are in

> place and there is no need to rename the parameter of the nested
> lambda.
> I changed the code slightly to overcome this scenario:
>
> protected override Expression VisitParameter(ParameterExpression p)
>        {
>            if (p.Type == _newParam.Type)
>                return _newParam;
>            else
>                return p;
>        }

You should really compare to the old parameter by reference instead.
Otherwise you'll always have edge cases not handled correctly.

Regards,
Fabian

[Variant]

Thanks for the comments.
Just a side note, the code I linked to is not mine. Just something I
found when I encountered the same problem with the PreidcateBuilder in
NH3.
For my existing scenario it works after my fix but I can clearly see
why it can easily break in a different scenario.

Thanks,
Ilan

[Fabian Schmied]

> Thanks for the comments.
> Just a side note, the code I linked to is not mine. Just something I
> found when I encountered the same problem with the PreidcateBuilder in
> NH3.

Sorry, my mistake. I've also posted a comment to that blog, so the
author should be aware of the issues by now.

Fabian