nhibernate-core
nhibernate-core copied to clipboard
nhibernate
Collection filter on joined subclass
Hello,
there is an problem with the generation of SQL-Code when a collection filter is set on a joined subclass. Here is an example mapping (modified existing joined-subclass.hbm.xml).
<?xml version='1.0' encoding='utf-8'?>
<hibernate-mapping assembly='NHibernate.Test'
namespace='NHibernate.Test.SubclassFilterTest'
xmlns='urn:nhibernate-mapping-2.2'>
<class name="Person" table="JPerson">
<id name="Id" column="person_id">
<generator class="native"/>
</id>
<property name="Name"/>
<property name="Company"/>
<property name="Region"/>
<joined-subclass name="Employee">
<key column="person_id" />
<property name="Title"/>
<property name="Department" column="dept"/>
<many-to-one name="Manager" column="mgr_id" class="Employee" cascade="none"/>
<many-to-one name="SharedCar"/>
<set name="Minions" inverse="true" lazy="true" cascade="all">
<key column="mgr_id"/>
<one-to-many class="Employee"/>
</set>
</joined-subclass>
<joined-subclass name="Customer">
<key column="person_id" />
<many-to-one name="ContactOwner" class="Employee"/>
</joined-subclass>
<filter name="region" condition="Region = :userRegion"/>
</class>
<class name="Car">
<id name="Id" column="car_id" >
<generator class="native"/>
</id>
<property name="LicensePlate"/>
<bag cascade="all-delete-orphan" inverse="true" name="Drivers">
<key>
<column name="SharedCar" />
</key>
<one-to-many class="Employee" />
<filter name="region" condition="Region = :userRegion"/>
</bag>
</class>
<filter-def name="region">
<filter-param name="userRegion" type="string"/>
</filter-def>
</hibernate-mapping>
Test case to display the issue:
JoinedSubclassFilterTest::
[Test(Description = "Tests the joined subclass collection filter of a single table with a collection mapping " +
"on the parent class.")]
public void FilterCollectionJoinedSubclass()
{
using(ISession session = OpenSession())
using(ITransaction t = session.BeginTransaction())
{
PrepareTestData(session);
// sets the filter
session.EnableFilter("region").SetParameter("userRegion", "US");
var result = session.Query<Car>()
.Where(c => c.Drivers.Any())
.ToList();
Assert.AreEqual(1, result.Count);
t.Rollback();
session.Close();
}
}
The filter on the collection mapping of the Car entity targeting a joined subclass causes this issue. The property is defined in the base class of the inheritance relation.
But there is a difference in SQL generation between your Master (developing branch?) and your 5.3. (stable?) branch.
Master:
-- Statement #1
select
car0_.car_id as car1_4_,
car0_.Tenant as tenant2_4_,
car0_.LicensePlate as licenseplate3_4_
from
car car0_
where
(
car0_.Tenant & 1 /* @p0 */
)
!= 0
and
(
exists (
select
drivers1_.person_id
from
inner join
(
empl drivers1_
inner join
person drivers1_1_
on drivers1_.person_id=drivers1_1_.person_id
)
on car0_.car_id=drivers1_.SharedCar
and
(
drivers1_1_.Tenant & 1 /* @p0 */
)
!= 0
)
)
-- Statement #2
ERROR:
Could not execute query: select car0_.car_id as car1_4_, car0_.Tenant as tenant2_4_, car0_.LicensePlate as licenseplate3_4_ from car car0_ where (car0_.Tenant & @p0) != 0 and (exists (select drivers1_.person_id from inner join (empl drivers1_ inner join person drivers1_1_ on drivers1_.person_id=drivers1_1_.person_id) on car0_.car_id=drivers1_.SharedCar and (drivers1_1_.Tenant & @p0) != 0))
Incorrect syntax near the keyword 'inner'.
--//////////////////////////////////////////////////
It seems there is a issue with join generation.
5.3:
-- Statement #1
select car0_.car_id as car1_3_,
car0_.LicensePlate as licenseplate2_3_
from Car car0_
where exists ( select drivers1_.person_id
from Employee drivers1_
inner join JPerson drivers1_1_
on drivers1_.person_id = drivers1_1_.person_id
where car0_.car_id = drivers1_.SharedCar
and drivers1_.Region = 'US' /* @p0 */ )
-- Statement #2
ERROR:
Could not execute query: select car0_.car_id as car1_3_, car0_.LicensePlate as licenseplate2_3_ from Car car0_ where exists (select drivers1_.person_id from Employee drivers1_ inner join JPerson drivers1_1_ on drivers1_.person_id=drivers1_1_.person_id where car0_.car_id=drivers1_.SharedCar and drivers1_.Region = @p0)
Invalid column name 'Region'.
--//////////////////////////////////////////////////
There is an issue evaluating the proper table alias.
The fix targets the 5.3 branch and tries to forward the filter generation to the underlying entity persister:
AbstractCollectionPersister::
/// <summary>
/// Get the where clause filter, given a query alias and considering enabled session filters.
///
/// When an <see cref="IEntityPersister"/> is set, the filter generated by the persister is returned. This ensures
/// that a proper filter is also generated for subclass hierarchy relations.
/// </summary>
/// <param name="alias"></param>
/// <param name="enabledFilters"></param>
/// <returns></returns>
public virtual string FilterFragment(string alias, IDictionary<string, IFilter> enabledFilters)
{
if(!enabledFilters.Any())
{
// no filters enabled
// but a potential where condition needs to be handled properly
return FilterFragment(alias);
}
if(!FilterHelper.IsAffectedBy(enabledFilters))
{
// the collection filters don't match with the enabled filters
// but a potential where condition needs to be handled properly
return FilterFragment(alias);
}
if(ElementPersister != null &&
ElementPersister.FilterHelper.IsAffectedBy(enabledFilters))
{
// forward filter generation to persister when it is affected by the filter
return ElementPersister.FilterFragment(alias, enabledFilters);
}
// default handling
StringBuilder sessionFilterFragment = new StringBuilder();
FilterHelper.Render(sessionFilterFragment, alias, enabledFilters);
return sessionFilterFragment.Append(FilterFragment(alias)).ToString();
}
Although it works, maybe there is a better way to evaluate the necessity to forward the generation call to the persister.
BR Michael
After the Master-Merge the new Testcase fails now with the already mentioned error:
NHibernate.Exceptions.GenericADOException : could not execute query
[ select car0_.car_id as car1_3_, car0_.LicensePlate as licenseplate2_3_ from Car car0_ where exists (select drivers1_.person_id from inner join (Employee drivers1_ inner join JPerson drivers1_1_ on drivers1_.person_id=drivers1_1_.person_id) on car0_.car_id=drivers1_.SharedCar and drivers1_1_.Region = @p0) ]
[SQL: select car0_.car_id as car1_3_, car0_.LicensePlate as licenseplate2_3_ from Car car0_ where exists (select drivers1_.person_id from inner join (Employee drivers1_ inner join JPerson drivers1_1_ on drivers1_.person_id=drivers1_1_.person_id) on car0_.car_id=drivers1_.SharedCar and drivers1_1_.Region = @p0)]
----> System.Data.SqlClient.SqlException : Incorrect syntax near the keyword 'inner'.
I don't know how to fix this one. I don't think it is related to my fix, cause the new test case also fails with this message without my changes.
Yes, as I mentioned initially, the fix targets the 5.3 branch. The Master branch (i guess it's you current development branch) causes further issues at generating proper SQL in case of joins.
the fix targets the 5.3 branch
The PR was pointing to the master branch, so I got confused.
Another question @micmerchant. Did it work before?
I guess, we are both confused now.
I branched from here (but I haven't really thought about it, why I've started from here :) )
But the point is .. the following test causes different issues in Master and in 5.3.x with the above mapping:
[Test(Description = "Tests the joined subclass collection filter of a single table with a collection mapping " +
"on the parent class.")]
public void FilterCollectionJoinedSubclass()
{
using(ISession session = OpenSession())
using(ITransaction t = session.BeginTransaction())
{
PrepareTestData(session);
// sets the filter
session.EnableFilter("region").SetParameter("userRegion", "US");
var result = session.Query<Car>()
.Where(c => c.Drivers.Any())
.ToList();
Assert.AreEqual(1, result.Count);
t.Rollback();
session.Close();
}
}
1) 5.3.x: proper SQL-Syntax is generated, but the query isn't executable, because the wrong table alias is resolved.
ERROR:
Could not execute query: select car0_.car_id as car1_3_, car0_.LicensePlate as licenseplate2_3_ from Car car0_ where exists (select drivers1_.person_id from Employee drivers1_ inner join JPerson drivers1_1_ on drivers1_.person_id=drivers1_1_.person_id where car0_.car_id=drivers1_.SharedCar and drivers1_.Region = @p0)
Invalid column name 'Region'.
2) Master: invalid SQL-Syntax is generated; there is some kind of a Join SQL problem.
ERROR:
Could not execute query: select car0_.car_id as car1_4_, car0_.Tenant as tenant2_4_, car0_.LicensePlate as licenseplate3_4_ from car car0_ where (car0_.Tenant & @p0) != 0 and (exists (select drivers1_.person_id from inner join (empl drivers1_ inner join person drivers1_1_ on drivers1_.person_id=drivers1_1_.person_id) on car0_.car_id=drivers1_.SharedCar and (drivers1_1_.Tenant & @p0) != 0))
Incorrect syntax near the keyword 'inner'.
My fix targets the first issue with resolving the wrong table alias.
I hope that clears it a bit up.
BR, Michael
It's not a regression so it's not applicable for 5.3 branch. You should target master branch. And master SQL syntax problem needs to be investigated anyway.
I believe I fixed issue with invalid SQL syntax on master in #3106. I also reproduced your issue using existing test data (see JoinedSubclassFilterTest.FilterCollectionWithSubclass2)
I finally found the time to merge the current Master (5.4) into this Branch. Yes, it seems that the SQL generation issues are fixed and at least my test cases are working.
But my changes don't fix the alias resolution for your mapping changes (@bahusoid), what you also mention in one of the test cases. I'm not sure if got the time to take a look at it or if i have enough knowledge about the inner working of NHibernate to fix it.
But, what I know is, if my fix doesn't fix all cases .. then it's not a real fix :)
Honestly, I initially thought that the filter name of a collection filter must match the entity filter name. In our application this is the case, but we are using the session filters as a tenant filter which is applied to all entities. In your changed mapping the filter names don't match, but if you change them to an equal name
the filter is properly applied but it could lead to a different semantic.
In this example there are at least two options:
- Do you want to load managers from the US with employees from the US?
- Do you want to load managers where only the employees are from the US?
So i guess, my fix doesn't work for the latter case.
Additionally the filter is not applied to the lazy loading of minions itself .. but I don't know where that lazy evaluation happens in code.
After taking a shower and rethinking the issue I came to the conclusion, that the mentioned issues may still be solved:
- lazy evaluation of collection:
Minions.Count failed because the minions were loaded from the session. I haven't cleared the session after data creation and before loading the data. After clearing the expected 2nd query was executed and the test case was successfull.
- different semantics: Well, honestly I don't think session filters are appropriate for this use case. I believe Linq filters or similar are way more flexible and appropriate.
What if you point out the limitations of session filters and their suitable areas of application in the documentation? A quick note about inheritance mappings and collection mappings should do it :). Of course only if you accept the PR which makes a matching session filter name in such mappings mandatory. I think a documentation update about the limitations is necessary anyway.
Collection filters are separate from entity filters. The fact that your fix only works if the same entity filter exists makes your fix invalid.
Legit.
Now that I understand a little bit more, I'll rethink it and try to find an appropriate fix.
Update: I've decoupled the generation of the filter code from the EntityPersister now. The EntityPersister just provides the column to table alias mapping anymore. So for collections and entities, the same logic to resolve the proper table alias for the filter generation is used.
@bahusoid
How to resolve "Changes requested" state? I guess, i was too fast with requesting another review, before i resolved the conversation :).
