NH-3845 - Make OfType work better with polymorphic mappings #456
Conversation
Addresses NH-3845. The ProcessOfType Linq visitor class now handles interfaces and base classes by checking which mapped types that could be in the result set implement the class, then filtering accordingly. If no results are possible, any remaining filters are removed from the query tree to avoid problems with invalid property names. Because the HQL tree for OfType can now mimic the ".class IN" construct, the In method of HqlTreeBuilder was inadequate to express the tree. Allowing an array of nodes to be passed as the source solves this problem.
|
I believe that you need to use ExpressionList instead of modifying In |
|
I tried using ExpressionList, and it causes an exception at parse time. When using the HQL construct |
| { | ||
| public virtual int MainEntityId { get; set; } | ||
| public virtual string Text { get; set; } | ||
| public virtual ISet<IPropertyEntityBase> Properties { get; protected set; } = new HashSet<IPropertyEntityBase>(); |
There was a problem hiding this comment.
C# 6 is not yet supported by our build engine, please change to using C#5
|
Ok, understand. |
|
I found another facet of the same bug, and I'm likely to be amending the pull request once I can create a minimal test case for it. |
Continues to address NH-3845. Because OfType is one of the result operators removed from its subquery and attached to the main query, it is not always clear during the ProcessOfType.Process method which query source should be used to retrieve the .class property of the entity. Sometimes the query type is in the QueryModel.MainFromClause, and sometimes it is in the QueryModel.SelectClause. It may be elsewhere, also. For queries in which OfType will match some but not necessarily all results, attempt to determine and use the query name as an identifier rather than using the source expression, for example. Doing so simplifies this particular Process method while still allowing query source determination later.
|
You will also need to check the caching issues. Please check that query do process correctly with regardless the type you specify. |
|
@hazzik, I'm not entirely sure what you mean when referring to "check the caching issues." Do you have a test case or example of some kind? Or, can you tell me where to look for a similar unit test? |
|
Start exploring from NH-2500 in JIRA. I can provide more resources later |
Two additional test methods check whether the changes for NH-3845 cause problems with query parameters being cached.
|
I added two unit test methods to check for cache issues similar to those NH-2500 describes. The tests pass. If there are other scenarios I should check, please let me know. |
|
I'm having some issues with another test case that I found does not pass. I posted to the nhibernate-developers Google Group, so see discussion there once my post is released from moderation. |
|
The Google Group thread has one reply, but more information would be very helpful. |
|
Happy birthday, PR! |
hazzik
changed the title
| @@ -416,7 +416,7 @@ public HqlFalse False() | |||
| return new HqlFalse(_factory); | |||
| } | |||
|
|
|||
| public HqlIn In(HqlExpression itemExpression, HqlTreeNode source) | |||
| public HqlIn In(HqlExpression itemExpression, params HqlTreeNode[] source) | |||
There was a problem hiding this comment.
I've found the proper solution for not changing this. You need to use HqlTreeBuilder.ExpressionSubTreeHolder.
| if (persister != null) | ||
| { | ||
| fromItemTypePersisters.Add(persister); | ||
| persister.EntityMetamodel.SubclassEntityNames.ToList().ForEach(fromItemImplementorQueue.Enqueue); |
There was a problem hiding this comment.
Please do not use .ForEach. Use the proper loop instead.
|
I've extracted the issue with unmapped base classes and interfaces to a new feature request NH-3947. |
|
Can you please enable Allow edits from maintainers option on this PR? |
Addresses NH-3845. The ProcessOfType Linq visitor class now handles interfaces and base classes by checking which mapped types that could be in the result set implement the class, then filtering accordingly. If no results are possible, any remaining filters are removed from the query tree to avoid problems with invalid property names.
Because the HQL tree for OfType can now mimic the ".class IN" construct, the In method of HqlTreeBuilder was inadequate to express the tree. Allowing an array of nodes to be passed as the source solves this problem.