Re: [hibernate-dev] Joins over type variable defined assocations NPE problems

2018-03-14 Thread Christian Beikov
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  > 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
> >
> 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 

Re: [hibernate-dev] Joins over type variable defined assocations NPE problems

2018-03-13 Thread 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 
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 
> 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@lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
> ___
> hibernate-dev mailing list
> hibernate-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Re: [hibernate-dev] Joins over type variable defined assocations NPE problems

2018-03-12 Thread andrea boriero
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 
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@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev