Hi Abe,
I'm guilty of not updating 245 after I finished it. The scenario that got me
to look into it is the edge case you mentioned - the entity has no auto
generated fields.
Improving the story for other scenarios sounds good to me and I like the
approach.
My main concern is that we don't break the edge case I mentioned. Something
like this should still work :
class Person {
@Id
int id;
String name;
// setters etc.
}
Person p = new Person();
p.setId(1);
p.setName("Mike");
em.persist(p);
// commit tran
// later
p = new Person(); // new entity - matches an existing row.
p2.setId(1);
p.setName("NotMike");
em.merge(p);
// commit tran
Under proposal #1 I'd have to add something like this to persistence.xml
<property name="openjpa.DetachState"
value="fetch-groups(DetachedStateField=false)"/>
With proposal #2 I wouldn't have to do anything, but it improves performance
for a smaller set of users.
I'm in favor of the first approach. Many users who want this kind of
processing will set the openjpaDetachState property to change loaded to
fetch-groups or all (otherwise you lose the ability to set a field to null).
On Fri, Apr 4, 2008 at 12:22 PM, Abe White <[EMAIL PROTECTED]> wrote:
> OpenJPA typically uses an enhancer-added "detached state field" to
> differentiate newly-constructed instances that need to be inserted vs.
> detached instances that need to be updated on merge. This is covered in
> more detail in the user manual.
>
> OpenJPA also allows users with version fields to manually construct a
> "detached" instance just by setting the pk fields to the pks of an
> existing instance and setting the version field appropriately. A
> non-default version field value is considered a directive to treat an
> instance as existing on merge, even if the detached state is missing.
>
> The OPENJPA-245 JIRA issue describes a case in which a user wants to
> manually construct a "detached" instance instead of going through the
> detach procedure, but the user's entity doesn't have a version field. A
> fix was submitted in January which basically changes our merge algorithm
> to always issue a database query to see if an instance with the same pks
> exists when no detached state is present.
>
> This has a few side effects:
> 1. This fix makes merging much more database-intensive, as we have to
> issue queries for all instances without detached state. Though it might
> not matter to OpenJPA devs, JDO actually treats persisting and merging
> the same, so the inefficiency is even more pronounced in JDO.
> 2. With this fix, the results of OpenJPAEntityManager.isDetached (and
> the methods it relies on like PersistenceCapable.pcIsDetached and
> Broker.isDetached) are no longer considered reliable -- the
> VersionAttachStrategy doesn't trust these results, but does its own
> lookup in the DB.
> 3. The fix changes the place that many exceptions will be thrown from
> flush/commit to merge. This is allowed by the spec and is arguably
> better (exceptions are thrown earlier), but I just want to point out
> that it is a change.
>
> I think we can fix 245 in a way that lessens the side effects. A couple
> options:
>
> 1. There is existing code in PersistenceCapable.pcIsDetached to consider
> an instance detached if it has any auto-assigned pk fields that aren't
> set to their default values. Right now this code is only executed if
> the user has configured things to never use a detached state field. We
> could easily change things, however, so that a non-default auto-assigned
> pk field is treated like a non-default version field: it always signals
> a detached instance, even when detached state is enabled but missing.
> This fixes 245 (which happened to involve an entity with auto-assigned
> pk fields) and removes all 3 side effects above.
>
> The downside is that if you have an entity without a version field and
> without auto-assigned pk fields and you attempt to construct a new
> "detached" instance and merge it instead of actually going through the
> detach process, OpenJPA will (correctly, but not desirably) think the
> instance is new rather than detached.
>
> This is a pretty esoteric use case. But even in this case, users have
> to option of turning off the detached state field via configuration, and
> OpenJPA will fall back on doing a DB query for the pk fields to
> determine whether these instances are detached vs. new. So there is
> still a way to handle this case, you just have to set a config property.
>
> 2. Another option would be to change VersionAttachStrategy to only do a
> DB lookup for types that have no version field. This would alleviate
> the inefficiencies for users that have version fields. Unfortunately,
> it wouldn't change any of the side effects of the existing fix for any
> other users.
>
> ========
>
> I would vote for proposal #1 above. It returns us to very efficient
> merge behavior (and JDO persist behavior) while allowing most users
> (those with either version fields or auto-assigned pk fields) to still
> manually construct and merge "detached" instances. The only users left
> out of the fix are those who want to manually construct and merge
> "detached" instances without version fields or auto-assigned pk fields.
> Even these users can turn off detached state, though, to force us to do
> DB lookups when determining whether an instance is detached vs. new.
>
> Thoughts?
>
>
> Notice: This email message, together with any attachments, may contain
> information of BEA Systems, Inc., its subsidiaries and affiliated
> entities, that may be confidential, proprietary, copyrighted and/or
> legally privileged, and is intended solely for the use of the individual or
> entity named in this message. If you are not the intended recipient, and
> have received this message in error, please immediately return this by email
> and then delete it.
>
