HHH-12436 : Attempted to assign id from null one-to-one property #2609
Conversation
| @@ -156,7 +195,7 @@ public Object hydrate( | |||
|
|
|||
| @Override | |||
| public boolean isNullable() { | |||
| return foreignKeyType==ForeignKeyDirection.TO_PARENT; | |||
There was a problem hiding this comment.
So my understanding of the isNullable method and how it is used is: can we safely assume, that a non-null identifier or unique key value for this association, will yield a non-null value and can it therefore be proxied?
For ManyToOnes this is rather trivial, as they are never mapped by a PrimaryKeyJoinColumn, but rather a JoinColumn, which is assumed to be constrained by default, which means that values must be present in the referenced table. The only exception being legacy schema's, with different magic values representing missing values. In this case you specify the NotFound action with regard to what should happen when an entity is not found under its identifier or unique key value.
For OneToOne's this is somewhat more complicated. First, apparently, it seems in the end the NotFound annotation was ignored for OneToOne associations (something I had already indicated here). But another case comes with the PrimaryKeyJoinColumn. Even though there will always be a value for the primary key, we're not sure whether the row in our referenced table exists. This is indicated by the constrainedness or optionality of the OneToOne association. It is in my opinion irrelevant to check this based on the foreign key direction. In fact: we have seen examples with bidirectional non-optionality. It can't be persisted with this cyclic dependency , but its certainly valid for some domains to assume that a value will always be there after its persisted and sensible to support this case for lazy loading.
Therefore, I am adamant that the nullability should be determined based on the constrained value and not the foreign key direction. This change must be applied before my test for issue HHH-12842 passes.
I've rebased my changes from #2544 on your work: https://github.com/JWGmeligMeyling/hibernate-orm/pull/new/HHH-12842-fix-4 , so that we can combine our two PRs.
There was a problem hiding this comment.
@jwgmeligmeyling, thanks very much for your feedback. I'll checkout your PR, then address the statements you made above.
There was a problem hiding this comment.
@jwgmeligmeyling, also, please rest assured that I fully intend to resolve your issue for 5.4.0.GA. I'm sorry I didn't have a chance to run your test before pushing the PR on Friday, and I'm sorry to hear that it didn't work in your case.
Also, please know that I had a long, hard look at a lot of the related code and there are very well-thought out reasons for why I addressed this the way I did. That said, I certainly could have missed something in my reasoning.
There was a problem hiding this comment.
Hi @gbadner , I know that if this issue was easy to solve, the two of us would have figured it out much sooner already. I see the hard work you've done on this (also apparent by the extensive number of test cases added in this PR), and I really appreciate your efforts! I don't want to exclude the possibility of my implementation having some new flaws, but I certainly gave it a lot of thought and research too.
Looking forward to your findings with my changes applied to your PR 😄
| // An optional @OneToOne with @PKJC or @PKJCs has an implicit @NotFound(IGNORE) | ||
| // (see HHH-4982). | ||
| if ( hasPkjc && !mandatory ) { | ||
| ignoreNotFound = true; |
There was a problem hiding this comment.
Perhaps this workaround is not needed after the alternative fix for the isNullable() method?
|
Superseded by #2612 |
https://hibernate.atlassian.net/browse/HHH-12436
Supersedes: #2226
Another jira will be created to deal with associations that change from FetchMode.JOIN to FetchMode.SELECT.