3:35 p.m.
Hi Steve,
I'm circling back to this. Please see below...
On Sat, Sep 2, 2017 at 8:47 AM, Steve Ebersole <steve(a)hibernate.org> wrote:
I don't have the original email to reply to. So I'll reply
here.
Overall, I had not really considered the primitive attribute case, but yeah
that's clearly an issue. My mistake.
A) I agree that the con here is huge. Not the best option
B) Is close to better. We could certainly check this and throw an error.
Throwing an exception is option A.
A better logical option is to do similar to what we do for ids and
versions...
on startup instantiate one of these and grab/store its initial state when
freshly instantiated. We can later use those values to perform the empty
check. This is more logical, but not sure how "practical" it is given that
we do not really have a good place to keep this relative to an embeddable,
nor relative to an embedded, aside from CompositeType, but that does not
feel right.
Is this what you refer to as (B) (expanded)?
My first email in the thread (which I told you to disregard) basically
suggests doing this. I told you to disregard it because I realized
that it would have serious issues.
Please see an example of the consequences when the embedded represents
an ID:
https://github.com/hibernate/hibernate-orm/pull/1993/commits/329354bfa4bd...
This is better in 6 as we have an actual runtime model
representation of the embedded and embeddable - but of course that does not
help us in 5
C) I really hate exposing `ComponentType` on a new SPI interface considering
the type system is completely revamped in 6. This would be (1) a very short
lived contract in this form and therefore (2) means yet another pain point
for user upgrading 5->6. Ultimately I think this is the most promising
solution moving forward (possibly coupled with the "expanded" B option)..
However, that said, I am not sure we have a choice prior to 6 if we want to
go this route - we'd have to expose the CompositeType.. There is no good
singular thing prior to 6 to describe a embedded/embeddable. To clarify
what sounds like a misunderstanding though, CompositeType is unique to each
embedded usage, not embeddable. The CompositeType however does not expose
its "role", so not sure if that distinction helps here.
Thanks for mentioning this. I'd forgotten that CompositeType is for
the "embedded" (JPA meaning) type.
If it's possible to get the role set in ComponentMetamodel, then it
seems we could use the role instead of the ComponentType in
EmptyCompositeStrategy SPI. H5 could determine the Type using the
role. Would that work in H6 as well?
(I noticed I had a redundant argument (Object component) in
EmptyCompositeStrategy#isEmptyComposite; it's removed below)
public interface EmptyCompositeStrategy {
/**
* Should a composite/embeddable be instantiated when a null attribute
* having the specified role is read?
*
* @return true if Hibernate should instantiate a composite/embeddable
* object when a null composite attribute is read.
*/
boolean supportsEmptyComposite(String role);
/**
* Is the specified value an empty composite that should be considered
* equal to <code>null</code>?
*
* @return true if the specified value is an empty composite.
*/
boolean isEmptyComposite(String role, Object value,
SessionFactoryImplementor factory);
}
To make sure I understand your (D)... you mean to allow users to disable
this option globally but to allow this option per specific embedded? Longer
term (6) I think this is something else we ought to support as well as the
inverse (globally opt-in, but allowing a local opt-out).
This is what I was suggesting, although I see I had a typo. It should have said:
D) Provide a way to opt-in when
hibernate.create_empty_composites_strategy=false, or to opt-out when
hibernate.create_empty_composites_strategy=true.
That's not necessarily a strategy though for dealing with
embeddeds that are "opted-in"
that happen to use primitives. Yes, it allows the user to more actively and
selectively manage that themselves, but if they happen to opt-in an embedded
which contains primitives we have the same issue to deal with.
I'm not sure I'm following this.
In the case where an application opts-in using
hibernate.create_empty_composites_strategy=true, and there is an
embeddabIe with primitive or non-null initialized values, I think it
would be reasonable to throw an exception as described by (A).
Longer term I see allowing a mix of (B) (expanded), (C) and (D).
For the short term (5), I think a the (possibly expanded) (B) option is the
best. I say that because (a) I prefer to not add a new contract like this
for a bug-fix release and (b) the concern about the immediate contract
change in 6. If we all deem that this is acceptable, I's be fine with (C)
as well.
If I'm understanding what you mean by (B) (expanded), then I don't
think that's is a good idea.
I think C is a good starting point in H5, preferably using role (if
that can be done).
Thanks,
Gail
On Fri, Sep 1, 2017 at 4:14 PM Gail Badner <gbadner(a)redhat.com> wrote:
>
> Hi Steve,
>
> No, I didn't hear back from you.
>
> I asked for a response to an email sent to hibernate-dev mailing list with
> subject, "HHH-11898: more "empty" composite issues".
>
> You can ignore the first message and just read the 2nd one.
>
> Thanks,
> Gail
>
> On Fri, Sep 1, 2017 at 12:53 PM, Steve Ebersole <steve(a)hibernate.org>
> wrote:
>>
>> You asked me to comment on an email, I'm sorry but I don't remember if I
>> did. Did I respond to you?
>
>