Hi Christian, I have looked at your PR and at the moment I cannot find another easy clean solution for this issue not requiring as you said quite a lot of work. On 9 March 2018 at 20:20, Christian Beikov <christian.beikov(a)gmail.com&gt; wrote: > Hey, > > so the problems related to the type variable handling I did aren't fully > solved. There is one other case that was reported today: > https://hibernate.atlassian.net/browse/HHH-12375 > > I'm not sure how to handle this. Let me explain you the model and the > problem. > > @Entity class BaseEntity { @Id Integer id; } > @Entity class EntityA extends BaseEntity { @ManyToOne EntityC relation; } > @Entity class EntityB extends BaseEntity { @ManyToOne EntityD relation; } > @Entity class EntityC {...} > @Entity class EntityD {...} > > The essence is, that EntityA and EntityB define properties with the same > names but have unrelated types. It's actually the same problem for > collections. > > Since EntityA and EntityB extend BaseEntity and because BaseEntity is an > @Entity, the Hibernate code tries to add "EntityA.relation" and > "EntityB.relation" to the PropertyMapping of BaseEntity. Before my fix, > the property which is added first won and subsequent adds are ignored. > This is presumably done to support implicit downcasting. My fix was > about making use of a common super type if possible and disallowing > ambiguous access. > > The problem the user now reported is, when using FetchType.EAGER there > is an exception which states that property "relation" could not be found > on type "BaseEntity". This is because I register "null" as type to > signal that it's not really possible to use that property with that > persister(BaseEntity in this case). During booting, the load plan for > BaseEntity is built, for which the SQL is pre-generated. At this point, > the exception happens, because "null" is registered as type for the > property "relation". This will cause the property to be considered as > non-existant wrt getPropertyType. The code before my fix was essentially > broken, because it doesn't handle eager fetching properly due to the > "first property wins" strategy. A property might only be loaded through > a secondary query due to lazy loading rather than by the defined fetch > style. An implicit downcasted join using the property might lead to > non-deterministic results. > > One of the reasons for doing the fix, was to prevent the possibility of > being hit by the non-determinism. I had a mapping that was using the > type variable approach and I had to find out the non-determinism the > hard way. I'd like to prevent access to such conflicting attributes from > the top level type through implicit downcasting, so IMO the only real > solution is to exclude the conflicting attributes in the load plan. > > The "best" solution would be to introduce a synthetic type that > represents the union of the unrelated property types, but to me this > seems to be quite a lot of work. > > I have implemented the proposed solution here: > https://github.com/hibernate/hibernate-orm/pull/2190 > > We need to release this fix soon as the current state is broken for the > EAGER configuration. What do you think about the solution or the matter > in general? > > -- > > Mit freundlichen Grüßen, > ------------------------------------------------------------------------ > *Christian Beikov* > _______________________________________________ > hibernate-dev mailing list > hibernate-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

Thanks for the feedback. I tried removing the attribute from the PropertyMapping at first, but it didn't work out in the end. Not remembering the full details, but it had something to do with load plans trying to resolve the properties contained in the subclass propery closure against the super type. I didn't see a nice way to make this work, so I decided to register "null" and empty join columns for such unresolvable types. Now, in the load plan, I simply skip association attributes that have empty join columns. The fix passes the testsuite and the original reporter confirmed that the fix works for him. Mit freundlichen Grüßen, ------------------------------ *Christian Beikov* Am 13.03.2018 um 20:06 schrieb Steve Ebersole: I think it is ok to disallow implicit downcasting in cases where the downcast is non-deterministic. From you example domain, I think it is ok to disallow the implicit downcast in : `from BaseEntity e where e.relation.id = ...' with either a specific error ("cannot implicitly downcast BaseEntity to (EntityA, EntityB) due to named attributes being unrelated") or a general "un-resolved attribute" error. On option to address that would be to change your registering a null, to instead recognizing that the passed property fits this description in its PropertyMapping impls and throw such an exception. And I think as long as you remove it from that entity's/component's various attribute-info arrays it should handle the SQL generation too. If you like that approach better... On Mon, Mar 12, 2018 at 12:02 PM andrea boriero <andrea(a)hibernate.org&gt; wrote: > Hi Christian, > I have looked at your PR and at the moment I cannot find another easy > clean > solution for this issue not requiring as you said quite a lot of work. > > > > On 9 March 2018 at 20:20, Christian Beikov <christian.beikov(a)gmail.com&gt; > wrote: > > > Hey, > > > > so the problems related to the type variable handling I did aren't fully > > solved. There is one other case that was reported today: > > https://hibernate.atlassian.net/browse/HHH-12375 > > > > I'm not sure how to handle this. Let me explain you the model and the > > problem. > > > > @Entity class BaseEntity { @Id Integer id; } > > @Entity class EntityA extends BaseEntity { @ManyToOne EntityC relation; > } > > @Entity class EntityB extends BaseEntity { @ManyToOne EntityD relation; > } > > @Entity class EntityC {...} > > @Entity class EntityD {...} > > > > The essence is, that EntityA and EntityB define properties with the same > > names but have unrelated types. It's actually the same problem for > > collections. > > > > Since EntityA and EntityB extend BaseEntity and because BaseEntity is an > > @Entity, the Hibernate code tries to add "EntityA.relation" and > > "EntityB.relation" to the PropertyMapping of BaseEntity. Before my fix, > > the property which is added first won and subsequent adds are ignored. > > This is presumably done to support implicit downcasting. My fix was > > about making use of a common super type if possible and disallowing > > ambiguous access. > > > > The problem the user now reported is, when using FetchType.EAGER there > > is an exception which states that property "relation" could not be found > > on type "BaseEntity". This is because I register "null" as type to > > signal that it's not really possible to use that property with that > > persister(BaseEntity in this case). During booting, the load plan for > > BaseEntity is built, for which the SQL is pre-generated. At this point, > > the exception happens, because "null" is registered as type for the > > property "relation". This will cause the property to be considered as > > non-existant wrt getPropertyType. The code before my fix was essentially > > broken, because it doesn't handle eager fetching properly due to the > > "first property wins" strategy. A property might only be loaded through > > a secondary query due to lazy loading rather than by the defined fetch > > style. An implicit downcasted join using the property might lead to > > non-deterministic results. > > > > One of the reasons for doing the fix, was to prevent the possibility of > > being hit by the non-determinism. I had a mapping that was using the > > type variable approach and I had to find out the non-determinism the > > hard way. I'd like to prevent access to such conflicting attributes from > > the top level type through implicit downcasting, so IMO the only real > > solution is to exclude the conflicting attributes in the load plan. > > > > The "best" solution would be to introduce a synthetic type that > > represents the union of the unrelated property types, but to me this > > seems to be quite a lot of work. > > > > I have implemented the proposed solution here: > > https://github.com/hibernate/hibernate-orm/pull/2190 > > > > We need to release this fix soon as the current state is broken for the > > EAGER configuration. What do you think about the solution or the matter > > in general? > > > > -- > > > > Mit freundlichen Grüßen, > > ------------------------------------------------------------------------ > > *Christian Beikov* > > _______________________________________________ > > hibernate-dev mailing list > > hibernate-dev(a)lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > _______________________________________________ > hibernate-dev mailing list > hibernate-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev