[hibernate-dev] Possible to confirm that versioning is broken for PersistentSets?
Can anyone confirm if or add some insight to why versioning is broken? Since Jan. 06 (v3.1.1), there's been an unassigned issue HHH-1401 saying that versions are updated unnecessarily on merge. At least three other issues have been added with similar problems for delete and in general with PersistentSets. I just reconfirmed the failing testcase from 1401 for r10347. What's the story?? ~Josh The Openmicroscopy Environment ---[links]--- http://opensource.atlassian.com/projects/hibernate/browse/HHH-1401 http://opensource.atlassian.com/projects/hibernate/browse/HHH-1564 http://opensource.atlassian.com/projects/hibernate/browse/HHH-1660 http://opensource.atlassian.com/projects/hibernate/browse/HHH-1668 ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Possible to confirm that versioning is broken forPersistentSets?
No patch that I know of. More than happy to work on one, but like I said, any confirmation, insights, or discussion would be welcome first. ~Josh. Steve Ebersole wrote: Sorry, I must have missed the patch implementing the fix. Where do I find that again? -Original Message- Can anyone confirm if or add some insight to why versioning is broken? Since Jan. 06 (v3.1.1), there's been an unassigned issue HHH-1401 saying that versions are updated unnecessarily on merge. At least three other issues have been added with similar problems for delete and in general with PersistentSets. I just reconfirmed the failing testcase from 1401 for r10347. What's the story?? ~Josh The Openmicroscopy Environment ---[links]--- http://opensource.atlassian.com/projects/hibernate/browse/HHH-1401 http://opensource.atlassian.com/projects/hibernate/browse/HHH-1564 http://opensource.atlassian.com/projects/hibernate/browse/HHH-1660 http://opensource.atlassian.com/projects/hibernate/browse/HHH-1668 ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Possible to confirm that versioning is broken forPersistentSets?
Sorry, myself, if there's nothing more anyone can say to versions being broken. Without input, though, I'll assume it's easier to roll our own. Thanks, ~Josh Steve Ebersole wrote: Sorry, I must have missed the patch implementing the fix. Where do I find that again? -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Josh Moore Sent: Thursday, August 24, 2006 1:25 PM To: hibernate-dev@lists.jboss.org Subject: [hibernate-dev] Possible to confirm that versioning is broken forPersistentSets? Can anyone confirm if or add some insight to why versioning is broken? Since Jan. 06 (v3.1.1), there's been an unassigned issue HHH-1401 saying that versions are updated unnecessarily on merge. At least three other issues have been added with similar problems for delete and in general with PersistentSets. I just reconfirmed the failing testcase from 1401 for r10347. What's the story?? ~Josh The Openmicroscopy Environment ---[links]--- http://opensource.atlassian.com/projects/hibernate/browse/HHH-1401 http://opensource.atlassian.com/projects/hibernate/browse/HHH-1564 http://opensource.atlassian.com/projects/hibernate/browse/HHH-1660 http://opensource.atlassian.com/projects/hibernate/browse/HHH-1668 ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Possible to confirm that versioning is broken forPersistentSets?
Actually had meant doing this in application code. Glad I didn't start. Thanks to the patch on HHH-1668 from Koda Janh, I could get the HHH-1401 test passing. Had to add a protective if-clause around the write statement in PeristentSet.clear() similar to Koda's in PersistentSet.add()/addAll()/remove()/removeAll()/retainAll(). The same needs to be done for most of the PersistentCollection methods which use write. I've got additions to Koda's patch for clear() and PersistentList. I can take it farther for Map and co., but I saw, Steven, that you changed the fix version to 3.2.1 for the group of bugs; have you made the changes already or should I keep extending patch? A few questions : for all the PersistentCollections which hold a Collection implementation, these calls could be pushed up into AbstractPersistentCollection. Is it worth the effort? And finally, what to do with the Iterators? ~Josh. Max Rydahl Andersen wrote: On Tue, 29 Aug 2006 00:45:52 +0200, Josh Moore <[EMAIL PROTECTED]> wrote: Sorry, myself, if there's nothing more anyone can say to versions being broken. Without input, though, I'll assume it's easier to roll our own. If you by "roll our own" means make a fix for it then we would of course be interested in it. As with all existing issues we need to prioritize them and issues with patches has a very high tendency to be fixed before issues without patches. /max Thanks, ~Josh ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Possible to confirm that versioning is broken forPersistentSets?
Ok. The isDirty()/write()/add()/clearDirty() idiom explains why the
patch was producing NPEs in other tests. Otherwise, the only other
failures that seemed they could be related were from the optlock tests.
Two were failing earlier (see
http://opensource.atlassian.com/projects/hibernate/secure/attachment/12560/TEST-org.hibernate.test.optlock.OptimisticLockTest.txt)
but all are failing with the patch, e.g.:
Testcase: testOptimisticLockAllDelete took 0.134 sec
Caused an ERROR
Batch update returned unexpected row count from update [0]; actual row
count: 0; expected: 1
~Josh.
Steve Ebersole wrote:
I've made some changes here locally which attempt to group fixes to a
bunch of these seemingly related issues. So for PersistentSet.add()
(for example), I have:
public boolean add(Object value) {
Boolean exists = isOperationQueueEnabled() ?
readElementExistence(value) : null;
if ( exists == null ) {
boolean initiallyDirty = isDirty();
write();
boolean actuallyAdded = set.add( value );
if ( !actuallyAdded ) {
if ( ! initiallyDirty && isDirty() ) {
clearDirty();
}
}
return actuallyAdded;
}
else if ( exists.booleanValue() ) {
return false;
}
else {
queueOperation( new SimpleAdd(value) );
return true;
}
}
But I need to get time to properly test these changes before I commit
them.
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Possible to confirm that versioning is broken forPersistentSets?
Got the fixes in r10470. Thanks. I updated HHH-1668 -- PList and PBag still have issues (failing tests). I can add a patch if you don't have it local, Steve. The failing tests I had for PMap have apparently been fixed. I'm also having problems again with HHH-1401. Added a test (see link below), but basically it comes down to this: // ... test prelude Parent p = new Parent(); Child c = new Child(); p.getChildren().add( c ); c.setParent( p ); s.save( p ); // ... test postlude // ... new session Parent p2 = s.merge( p ); // ... test postlude assertTrue( p.getVersion().equals( p2.getVersion() ); // BOOM Is this the expected behavior that merging a collection will _always_ cause a version update?? --[links]-- http://opensource.atlassian.com/projects/hibernate/browse/HHH-1668 http://opensource.atlassian.com/projects/hibernate/browse/HHH-1401 http://opensource.atlassian.com/projects/hibernate/secure/attachment/12686/HHH1401Test.java ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Possible to confirm that versioning is broken forPersistentSets?
Max Rydahl Andersen wrote: if there are additions/removals to the collection then yes. The reason being that the actual collections are considered value types of the entity. If you don't want optimistic locking to fire here then set optimistic-lock="false" on the association. Thanks Max. The question being: and if there _weren't_ additions/removals? I perhaps cut out too much of the test I attached to HHH-1401, but after the first save everything is flushed. The merge is then getting a pure detached graph (no changes). It seems that CollectionType.replaceElements makes lots of calls to the PersistentCollection which force the dirty flag to be set. What I can't find is the logic that would check to see that this, in fact, not dirty and unset dirty. ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Possible to confirm that versioning is broken forPersistentSets?
Steve Ebersole wrote:
Without having had a chance yet to look, what exactly are the proposed
changes in regards to lists and bags?
The changes are identical to PersistentSet omitting the add* methods.
I.e. clear, remove, removeAll, and retainAll all use the idiom:
initialize(true)
if ( list.clear() )
{
list.clear() )
dirty();
return true;
} else {
return false;
}
As for the code snippet and question, you'd need to find out why the
collection is considered dirty on merge...
Ok. Good to know it's not expected behavior. I know that it's initially
dirty due to CollectionType.replaceElements usage of the
PersistenCollection rather than raw collection. And there are only two
places where AbstractPersistentCollection.clearDirty() is called:
(1) postAction -- this is too late, and
(2) new CollectionEntry() -- specifically (note the comment):
"public CollectionEntry(...
//...
collection.clearDirty(); //a newly wrapped collection is NOT dirty (or
we get unnecessary version updates)"
But these CollectionEntries are created in
DefaultMergeEventListener.entityIsDetached(231). Too early, really.
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
[hibernate-dev] RE: new SPI for EJB3/Seam (Gavin King)
Redirecting to list.jboss.org from sf.net For truly efficient clustering of extended persistence contexts in the context of Seam or EJB3, we need a way to propagate unflushed changesets. What we need is an SPI like this: Serializable changeset = session1.getChangeSet(); session2.applyChangeSet(changeset); where session2 is an empty session, or perhaps even a session with some of the same entities as session1. The only state that needs to be sent in the changeset is new entities, unflushed updates to entities and deletions of entities. Steve, WDYT? +1 even if my names not Steve. Seems like the same infrastructure would also be usable in implementing a remote client session. (Discussed in various places.) ~Josh. ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
[hibernate-dev] More fun with merging : non-updatable fields
Using Hibernate with non-updatable fields can leave entities in a confused state. Take an Image with a field creationEvent, not updatable. If Image.creationEvent is set on an instance and passed to session.merge(), then: (1) Properly, no UPDATE is issued. (2) DefaultMergeEventListener.copyValues copies the invalid creationEvent to the new instance (3) The client who receives that instance thinks creationEvent has been successfully updated. What are the semantics here? Should the spec specify this? Max, you mentioned via irc that this wasn't in the spec, but I assume @Column( updatable = false ) would cause the same issue. Currently I'm writing logic in my MergeEventListener to NOT copy these values (difficult because of the EmbeddedComponentTypes I'm using). It seems, however, that this should be belong directly to DefaultMergeEventListener. Opinions? ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] More fun with merging : non-updatable fields
Emmanuel, do you mean saveOrUpdateCopy? Since saveOrUpdate doesn't do any copying of the values onto another instance. By the way, the "dirtying" of the non-updatable field I described also holds for collections. This means that DefaultMergeEventListener does a source.load(), gets a fully valid object with proxied collections (which would later be lazy-loadable with the current values), copies _invalid_ values on top of the clean proxied collection, and sends that back to the user. Seems counter-productive. Thanks, Josh. Emmanuel Bernard wrote: This is consistent with the way saveOrUpdate works Josh Moore wrote: Using Hibernate with non-updatable fields can leave entities in a confused state. Take an Image with a field creationEvent, not updatable. If Image.creationEvent is set on an instance and passed to session.merge(), then: (1) Properly, no UPDATE is issued. (2) DefaultMergeEventListener.copyValues copies the invalid creationEvent to the new instance (3) The client who receives that instance thinks creationEvent has been successfully updated. What are the semantics here? Should the spec specify this? Max, you mentioned via irc that this wasn't in the spec, but I assume @Column( updatable = false ) would cause the same issue. Currently I'm writing logic in my MergeEventListener to NOT copy these values (difficult because of the EmbeddedComponentTypes I'm using). It seems, however, that this should be belong directly to DefaultMergeEventListener. Opinions? ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] More fun with merging : non-updatable fields
Thanks, Emmanuel. I'll ponder that tonight. And I'll try to convince myself that being consistent with saveOrUpdate is a good thing for merge, but I'm not hopeful. ;) Cheers, Josh. Emmanuel Bernard wrote: No I mean saveOrUpdate Think about how saveOrUpdate works in your case, and you will see that merge is very consistent. Josh Moore wrote: Emmanuel, do you mean saveOrUpdateCopy? Since saveOrUpdate doesn't do any copying of the values onto another instance. By the way, the "dirtying" of the non-updatable field I described also holds for collections. This means that DefaultMergeEventListener does a source.load(), gets a fully valid object with proxied collections (which would later be lazy-loadable with the current values), copies _invalid_ values on top of the clean proxied collection, and sends that back to the user. Seems counter-productive. Thanks, Josh. ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
[hibernate-dev] Re: More fun with merging : non-updatable fields
Sorry, Christian, didn't get your emails because of a delayed digest message. And I agree that it can be seen as reasonable behavior. With saveOrUpdate (as Emmanuel was pointing out) one gets * in DB: no change * in memory: (invalid) change and with these semantics for merge: * in DB: no change * in memory: (invalid) change. Understood. I think there's a case to be made, however, for not expending extra time for merge to make the in memory representation invalid. For me it's a part of what "merge" means : "Take the DB representation and the in-memory representation and produce something _new_". I.e. I'm expecting to have to deal with that situation. SaveOrUpdate differs in that I still have my in-memory object references, so no one should screw with them. Merge is polar opposite. I'm given new references and have to, somehow, deal with this new state. My opinion, which is nothing more than that, is : it'd be nice if those new object references were valid. But, *shrug*, whatever. If those aren't desired or generally useful semantics, I'll finish implementing the logic for my MergeEventListener. I could use some help getting hold of EmbeddedComponentType's metamodel (see http://opensource.atlassian.com/projects/hibernate/browse/HHH-1907). But I'll catch Max on irc for that. ;) Thanks again, Josh. P.S. one last thing, Christian. You mentioned "Don't [apply those values] if you don't want [them]", though there's one case where it's actually "Then do [appply those values] if you want [them]". Namely on the reverse side of a bidirectional many-to-one. If you call child.setParent(parent) and not parent.getChildren().add(child), then you'll get a parent back which still doesn't have children in it. A bit freaky the first time you experience it. And no, Hibernate doesn't do CMR, but it also needn't go out of it's way to NOT do CMR. Cheers. Christian wrote: This is consistent with the way saveOrUpdate works And it's perfectly reasonable behavior. If I modify a field value, then merge, then take the instance returned by merge, I expect that the value is still there in the merged result. That has nothing to do with the update/UPDATE behavior of Hibernate. copies _invalid_ values on top of the clean proxied collection, and sends that back to the user. It does so because you applied these values. Don't do that if you don't want it. Thanks to Hibernate your mistake will not end up in the database. Max wrote: Yes, Josh - consider especially the situation Christian explained that if you merge Xy onto a session you expect the returning object is Xy and not Xx. If you want this behavior (which in some cases I can see the usefullness of) you need to have a custom merge implementation as you already have. /max ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
