Problems with "degenerate" selects when eager fetching using Expand with DistinctRootEntityResultTransformer
Description
I encountered a bug when eager fetching collections in linq queries. I specified a DistinctRootEntityResultTransformer() on the criteria but in some cases, the results weren't being flattened to distinct entities.
Here are the failing tests (in NHibernate.Linq.Tests):
[Test] public void DegenerateSelectInMethodChainShouldNotReplaceResultTransformer() { var queryable = session.Linq<Animal>(); queryable.Expand("Children"); queryable.QueryOptions.RegisterCustomAction (c => c.SetResultTransformer(new DistinctRootEntityResultTransformer ()));
[Test] public void DegenerateSelectInLinqShouldNotReplaceResultTransformer() { var queryable = session.Linq<Animal>(); queryable.Expand("Children"); queryable.QueryOptions.RegisterCustomAction (c => c.SetResultTransformer(new DistinctRootEntityResultTransformer ()));
var animals = (from a in queryable select a).ToList(); Assert.AreEqual(6, animals.Count); }
The cause is the following method in CriteriaUtil, which is called by RootVisitor after visiting the Select call, the IResultTransformer that was originally set on the criteria gets replaced:
public static void SetResultTransformerIfNotNull(this ICriteria criteria, IResultTransformer transformer) { if (transformer != null) criteria.SetResultTransformer(transformer); else criteria.SetResultTransformer(new RootEntityResultTransformer()); }
I tried this first and all tests pass - feels a bit hacky though.
public static void SetResultTransformerIfNotNull(this ICriteria criteria, IResultTransformer transformer) { if (transformer != null) criteria.SetResultTransformer(transformer); else { SetEntityResultTransformer(criteria); } }
private static void SetEntityResultTransformer(ICriteria criteria) { var criteriaImpl = criteria as CriteriaImpl; if (criteriaImpl != null && criteriaImpl.ResultTransformer is DistinctRootEntityResultTransformer) { return; } criteria.SetResultTransformer(new RootEntityResultTransformer()); }
2. Add property to QueryOptions (something like bool SelectDistinctRootEntities) and make this available to RootVisitor so that it can work out what kind of IResultTransformer to use by default in call to SetResultTransformerIfNotNull. I'm leaning towards this one myself, it would make the selection of the correct IResultTransformer for entities more explicit.
3. Add a DegenerateSelectReducer that strips redundant Select calls out of the expression. Not sure if this addresses the core problem though.
I can supply a patch for 1 or 2. I'd have to pass on 3 right now
Thoughts? Any other possible approaches to a fix?
Environment
None
Attachments
1
Activity
Alex Zaytsev
September 3, 2014 at 11:08 PM
Just forget about this project It is [DEPRECATED] )
Ricardo Peres
September 3, 2014 at 12:01 PM
So let's close it?
Alex Zaytsev
September 3, 2014 at 1:55 AM
Yep, it is NHLQ project
Ricardo Peres
September 3, 2014 at 12:14 AM
This is using the original LINQ provider (session.Linq<T>). Probably does not make sense now.
Dan Malcolm
February 1, 2010 at 5:36 AM
Fix attached for proposed resolution option 1.
Would prefer some feedback before finalising best way to fix, but this might help in the interim.
I encountered a bug when eager fetching collections in linq queries. I
specified a DistinctRootEntityResultTransformer() on the criteria but
in some cases, the results weren't being flattened to distinct
entities.
Here are the failing tests (in NHibernate.Linq.Tests):
[Test]
public void
DegenerateSelectInMethodChainShouldNotReplaceResultTransformer()
{
var queryable = session.Linq<Animal>();
queryable.Expand("Children");
queryable.QueryOptions.RegisterCustomAction
(c => c.SetResultTransformer(new DistinctRootEntityResultTransformer
()));
var animals = queryable.Where(a => a.Id == 1).Select(a => a).ToList
();
Assert.AreEqual(1, animals.Count);
}
[Test]
public void DegenerateSelectInLinqShouldNotReplaceResultTransformer()
{
var queryable = session.Linq<Animal>();
queryable.Expand("Children");
queryable.QueryOptions.RegisterCustomAction
(c => c.SetResultTransformer(new DistinctRootEntityResultTransformer
()));
var animals = (from a in queryable select a).ToList();
Assert.AreEqual(6, animals.Count);
}
The cause is the following method in CriteriaUtil, which is called by
RootVisitor after visiting the Select call, the IResultTransformer
that was originally set on the criteria gets replaced:
public static void SetResultTransformerIfNotNull(this ICriteria
criteria, IResultTransformer transformer)
{
if (transformer != null)
criteria.SetResultTransformer(transformer);
else
criteria.SetResultTransformer(new RootEntityResultTransformer());
}
The "a => a" Select method call will always be included in the
expression when you use the method chain. It will "sometimes" be
included with linq queries (interesting article about "degenerate"
query expressions here:
http://blogs.msdn.com/ericlippert/archive/2008/05/12/trivial-projections-are-usually-optimized-away.aspx).
I've thought about some possible solutions:
1. Special case treatment in CriteriaUtil:
I tried this first and all tests pass - feels a bit hacky though.
public static void SetResultTransformerIfNotNull(this ICriteria
criteria, IResultTransformer transformer)
{
if (transformer != null)
criteria.SetResultTransformer(transformer);
else
{
SetEntityResultTransformer(criteria);
}
}
private static void SetEntityResultTransformer(ICriteria criteria)
{
var criteriaImpl = criteria as CriteriaImpl;
if (criteriaImpl != null && criteriaImpl.ResultTransformer is
DistinctRootEntityResultTransformer)
{
return;
}
criteria.SetResultTransformer(new RootEntityResultTransformer());
}
2. Add property to QueryOptions (something like bool
SelectDistinctRootEntities) and make this available to RootVisitor so
that it can work out what kind of IResultTransformer to use by default
in call to SetResultTransformerIfNotNull. I'm leaning towards this one
myself, it would make the selection of the correct IResultTransformer
for entities more explicit.
3. Add a DegenerateSelectReducer that strips redundant Select calls
out of the expression. Not sure if this addresses the core problem
though.
I can supply a patch for 1 or 2. I'd have to pass on 3 right now
Thoughts? Any other possible approaches to a fix?