Hi Abe, I like this approach better - since we already have at least one customer hitting the corner case. The caveats seem reasonable to me, but then again I'm not working on Kodo ;-).
Does anyone else object, or have other concerns? -Mike On Fri, Apr 4, 2008 at 3:41 PM, Abe White <[EMAIL PROTECTED]> wrote: > Here's another option that might suit everyone: > > - We change the enhancement so that a non-default auto-assigned pk field > makes an instance detached even in the absence of detached state, as I > proposed. > - If the entity doesn't use auto-assigned pk fields (and has no version > field) and detached state is in use but not present, > PersistenceCapable.pcIsDetached will now return null for JPA enhanced > entities. We already treat null as "I don't know", and it causes us to > go to the DB to see if there is an existing record. For JDO enhanced > entities, we'll return false as before. We can do this by returning the > result of a new private pcIsDetachedStateDefinitive method we'll add, > and the JDO enhancer will change the return value of this method from > null to false. This won't cause any compat problems with > already-enhanced entities, because it won't be part of the public > PersistenceCapable interface. > > This way the corner case you're concerned about is covered without > having to change the default config, and meanwhile JDO users still get > efficient persist behavior. > > The downsides are: > - Merging new version-less and auto-assigned-pk-less entities (i.e. ones > you really do want inserted, not updated, but are found during merge > instead of passed to persist) still causes a DB lookup to make sure > there is no existing entity with the same PKs. I think this is so > infrequent as to be perfectly OK. > - If a user upgrades Kodo without reenhancing OPENJPA-245 won't be > "fixed" for him. Then when he reenhances the behavior will change and > it will act fixed. Kind of odd, but I can live with it. > > > On Fri, 2008-04-04 at 14:26 -0500, Michael Dick wrote: > > 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). > > > 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. >