Wrong statement when child classes have same property name but different types

[FelicePollano]

I think there is something not working while generating a query in a hierarchy of classes in which some derived classes have an association property with the same name but different many-to-one class. ie:

   				+----------+
   				+  mother  +
   				+----------+
   			 
   			 
   +-------------+				+-------------+
   +    Child1   +				+    Child1   +
   ===============				===============
   | Thing:Thing1|				| Thing:Thing2|
   +-------------+				+-------------+

So if we look for a specific value of Thing property like this:

var result = session.Query<Mother>().Where(k => k is Child1 && (k as Child1).Thing.Id == "00001").ToList();

We do not find any record even if the record exists, as the query generated is left-joining the wrong table, ie:

select mother0_.Id as id1_0_, mother0_.Name as name3_0_,
		mother0_.thingId as thingid4_0_, mother0_.kind as kind2_0_ from Mother mother0_ 
	left outer join Thing2 thing2x1_ on 
		mother0_.thingId=thing2x1_.Id where mother0_.kind='1' and thing2x1_.Id=?"

That is, looks into table for Thing2, while we actually look for a Thing1...

Unit test in PR #3364

[bahusoid]

Looks like duplicate of #1189

[FelicePollano]

Actually can be same issue. I can add I had the case working in production until 5.2.7, it breaks after 5.3.0 included.

[fredericDelaporte]

So, it would be a 5.3.x regression. I consider checking this before releasing the next 5.4.x.

[bahusoid]

I had the case working in production until 5.2.7, it breaks after 5.3.0

I see it's about #1189 + not-found="ignore" mapping. In 5.2.7 join wasn't generated for (k as Child1).Thing.Id == "00001" check. But in 5.3 LEFT JOIN is generated to make sure that not found records are ignored in query too. And this gives us join on ambiguous name - #1189

I have no good fix for this case in mind - you better avoid using the same names in subclasses for different entities.

[FelicePollano]

I have no good fix for this case in mind - you better avoid using the same names in subclasses for different entities.

actually this is current solution adopted, but, if this is the case, some exception in mapping phase should complain, otherwise is easy to be confused.

[FelicePollano]

By looking at current code base, looks like there is something wrong here: ( AbstractPropertyMapping )

protected void AddPropertyPath(string path, IType type, string[] columns, string[] formulaTemplates)
{
			typesByPropertyPath[path] = type;
			columnsByPropertyPath[path] = columns;

			if (formulaTemplates != null)
				formulaTemplatesByPropertyPath[path] = formulaTemplates;
}

So this function is called collecting a property name/type map for the entire hierarchy of an entity, so, if down the entire tree some property have same name, entry will be overwritten, causing unexpected behaviors. I had issues compiling 5.2.7 tag because of missing requirement, but maybe in the previous version this was called only for single entity?

[fredericDelaporte]

No, I do not think this has changed. What has changed is #2081, fixing #1274.

Your mapping uses not-found="ignore" on the relation, which means a not null foreign key does not guarantee the associated entity exists. Prior to #2081, in case of comparison on the associated ID, NHibernate would do that on the foreign key without checking the associated entity actually exist, which could yield wrong results in case the entity was not actually existing.
Now it checks the associated entity existence, but is hit by #1189 in case of polymorphism.

(About issues for compiling, switching from 5.4.x to a lower version branch usually requires cleaning, closing the solution in VS, restoring. Actually, it is more reliable to close the solution after switching branch, purge all bin and obj folders, reopen the solution.)

So, by fixing a not-found="ignore" bug in some cases, #2081 has caused another bug (#1189) to be extended to some not-found="ignore" specific cases. By the way, your application was surely hit by the bug fixed by #2081, unless it does not actually have missing entities. If it does not have missing entities, remove not-found="ignore". It will solve the trouble.

[fredericDelaporte]

So, this is still a regression caused by #2081. But maybe a tough one to fix, hitting what is in my opinion a corner case (not-found="ignore" + polymorphism with different association properties sharing the same name). Not sure we should mandate a fix for this one in 5.3.x.

[bahusoid]

Not sure we should mandate a fix for this one in 5.3.x.

Agree. Not easy fixable. No need to wait

I see that in the test case the same column name thingid is used to reference two different tables (Thing2 and Thing1). If not-found="ignore" is added just to avoid foreign key creation - you should use foreing-key="none" mapping instead.

[FelicePollano]

Hi @bahusoid, no, it is not to avoid foreign key creation. By the way, even if not modified, from what I saw, the function AddPropertyPath mentioned above, looks like it is the source of the problem. In any hierarchy, if two properties share same name, and that could actually happen for any reason, the type map is overwritten by the last entry found. Here the problem. I suspect that can be causing some issues somewhere else as well. Short "fix", according to me, could be at least throw an exception when this happens, pointing to the undesired duplicated property name.

[fredericDelaporte]

Yes the issue is more general (and known since long), but complex to fix, see #1189.

Still, your specific case could be fixed by a "work-around": cease using that other "work-around" that is not-found="ignore".

(That ignore feature is a work-around for supporting incoherent data in the database. It is best to not use it if possible. This means, fix invalid data and enforce in the DB foreign key constraints to avoid new invalid data, then remove from the mapping these ignore settings.)