Hi Markus,
Comments inline
Markus Fuchs wrote:
>Hi Tom!
>
>[Including the persistence alias in the discussion]
>
>I hope you had a great vacation!
>
>While investigating issue 879 I noticed, that the class
>OneToManyMapping doesn't have to implement method
>hasInverseConstraintDependency().
>
>The dependencies corresponding to a OneToMany/ManyToOne relationship
>are always added by the owning side, which is represented by the class
>OneToOneMapping in TopLink, correct?
>
>So, the dependencies are added twice, once from the owning, and once
>from the non-owning side. It should be possible to remove this method,
>letting OneToManyMapping fall back to the default implementation
>returning 'false'. Would you agree?
>
>
If we make this change, how will the commit order calculation logic work
with we are processing a OneToMany? With the code as it is, it looks to
me like if we have OneToManyMapping return false from this method, in
recordMappingDependencies() we will get end up adding a dependancy that
is in the incorrect order (because we will get into the "if" rather than
the "else")
>Could you please also comment on my evaluation of issue 879? The
>change in class ManyToManyMapping could be the fix for this issue.
>
>
I'll add comments to the bug shortly.
>
>For issue 699, I changed UnitOfWorkImpl to remove the ObjectChangeSets
>for all objects that are to be deleted before updates/inserts are
>processed in any case. I got the impression that the updates for
>deleted objects are executed in order to prevent foreign key (unique
>key) violations. Is this correct?
>
>There's no database constraint forcing to nullify foreign keys for for
>OneToMany relationships, before other updates happen. The same is true
>for join table entires in case of ManyToMany relationships, which
>don't need to be removed. The only case where nulling out the foreign
>key is important is for OneToOne relationships, which might have a
>unique constraint on the foreign key in the database. But as
>dependencies within the same class can't be determinated anyway,
>removing those updates should not make a difference. Please see the
>attached UnitOfWorkImpl class for my change.
>
>
I am concerned about the change to UnitOfWorkImpl and will need to take
a closer look at the issue. Here is my concern: It looks like the way
we are solving this issue is to go through the change sets as usual and
build a change set as normal and them make a second pass to "fix" the
change set. Ideally, I think we would build a change set that we could
use correctly like from the beginning. I'll take a look at this issue
in some more depth and get back to you.
>I ran entity-persistence-tests w/o regressions with my changes. What
>do you think?
>
>Thanks very much,
>
>-- markus.
>
>
>Tom Ware wrote:
>
>
>
>>Hi Markus,
>>
>> I was on vacation, but it looks like you got your answer. Feel free
>>to contact me with questions.
>>
>>-Tom
>>
>>Markus Fuchs wrote:
>>
>>
>>
>>>Hi Tom,
>>>
>>>is it ok if I look into this?
>>>
>>>Thanks!
>>>
>>>-- markus.
>>>
>>>
>>>
>>>
--
Tom Ware
Principal Software Engineer
Oracle Canada Inc.
Direct: (613) 783-4598
Email: tom.ware_at_oracle.com