Hi Tom,
> 3. I am trying to figure out if there may be a spec issue here. i.e.
> Is it possible for the persistence provider to know enough about the
> underlying schema to know whether or not it can avoid an update that
> has been asked for by the user. Are there some times when the foreign
> key goes both ways in a bidirectional relationship where we just can't
> know if we can avoid that delete. I'll have to think about that one
> some more and get back to you if/when I come up with an answer.
>
Can this situation be detected at model construction time? If yes, we
could remember it in the ClassDescriptor, and it would prevent us from
skipping any change calculation (the property would be true in very rare
cases). If we then pass the information, if an object is being deleted
into the change detection, the field mappings could decide, if change
calculation is necessary, or not:
* it's not necessary for all non-relationship fields
* it's not necessary for OneToOneMappings representing ManyToOne mappings
* it's not necessary for ManyToManyMappings
* it might be necessary for the other non-abstract classes extending
ForeignReferenceMapping
What do you think?
Thanks,
-- markus.
> -Tom
>
> Markus Fuchs wrote:
>
>> Hi Tom,
>>
>> Tom Ware wrote:
>>
>>
>>
>>> 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")
>>>
>>>
>>
>>
>> I understand the handling of the two cases in
>> recordMappingDependencies()
>> as follows:
>>
>> (1) (ForeignReferenceMapping)mapping).hasConstraintDependency() == true
>>
>> This is for the owning side. The referenced class is the owned class
>> and is added with all its subclasses to the owner's list relatedNodes.
>>
>> (2)
>> ((ForeignReferenceMapping)mapping).hasInverseConstraintDependency()
>> == true
>>
>> This is for the non-owning side. The current class is the owned class
>> and
>> is added with all its subclasses to the owner's list of relatedNodes.
>>
>>
>> recordMappingDependencies() uses an exclusive if ("if" ... "else if")
>> construct. So if the two methods hasConstraintDependency() and
>> hasInverseConstraintDependency() are implemented consistently for each
>> mapping, there is no problem of adding "reversed" dependencies.
>>
>> The implementation of hasInverseConstraintDependency() for
>> OneToManyMapping is *correct*, and shouldn't be changed. Rather, one
>> could consider changing the implementation of
>> recordMappingDependencies() to evaluate only the owning side (case 1)
>> for calculating the dependencies. But it's not a big deal anyway. I
>> withdraw my proposal to change OneToManyMapping.
>>
>>
>>
>>>> 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.
>>>
>>>
>>
>>
>> Thanks, I'll wait for your update!
>>
>> -- markus.
>>
>>
>>
>>>> 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.
>>>>>>
>>>>>>
>>>>>>
>>>>>
>