I think Sanne meant relying on equals/hasCode of the embeddable itself, which we do not do as you pointed out.
On Wed, Dec 4, 2019 at 5:19 PM Gail Badner <gbad...@redhat.com> wrote: > Hi Sanne, > > By default, the cache key is of type CacheKeyImplementation [1]. As long as > a composite key is a ComponentType (not a CustomType), > CacheKeyImplementation#equals and #hashCode uses ComponentType#equals and > #hashCode, not the custom implementation of the embeddable class methods. > > IIRC, a CustomType cannot contain an association, so disassembling a custom > type should not be necessary. > > Steve, does that sound right to you? > > I believe that what you mentioned above does apply to "simple" cache keys, > where the cache key is the ID itself. There are other cases where a simple > cache key is not appropriate (e.g., multiple entity classes with the same > type of ID). I don't remember offhand if there are warnings logged in those > cases. We could warn if an application attempted to use a simple cache key > with a cacheable entity that has a composite key with an association. > > Regarding B -- it is not possible to assemble the ID when > calling CacheKeyImplementation#equals or #hashCode, because > a CacheKeyImplementation does not have a reference to a Session, so it > cannot resolve an associated entity. In any case, there should be no need > to assemble an associated entity. It should be sufficient to for the > disassembled entity ID to be used for #equals and #hashCode operations. > > [1] > > https://github.com/hibernate/hibernate-orm/blob/master/hibernate-core/src/main/java/org/hibernate/cache/internal/CacheKeyImplementation.java > > > > On Wed, Dec 4, 2019 at 2:03 PM Sanne Grinovero <sa...@hibernate.org> > wrote: > > > Hi Gail, > > > > going for a disassembled ID would seem logical, but we'll need some > > special care to deal with custom implementations of equals/hashcode. > > > > Clearly a composite ID object would require the users to implement a > > custom equals(); going for a solution based on a disassembled ID we > > would need to either: > > > > A# trust the equals implementation is "the obvious one" > > > > B# hydrate both sides of the equals check for each time there's need > > to invoke an equals (and same for hashcode) > > > > I'm afraid only B would be backwards compatible and bullet-proof, and > > yet its performance overhead would make it a terrible choice. > > > > Perhaps the best solution is to constrain this, and warn that such a > > model is not a good fit for caching? > > > > Unless someone has a better idea; thinking out of the box: could be > > interesting to explore not allowing users to implement custom equality > > contracts as that's the root of many problems, but that would require > > much more careful thought, and for sure a significant breaking change. > > > > Or allow people to implement any custom equals, but ignore them and > > apply "the obvious one" consistently. > > > > Thanks, > > Sanne > > > > > > > > On Wed, 4 Dec 2019 at 19:40, Gail Badner <gbad...@redhat.com> wrote: > > > > > > When an entity is cached with a composite ID containing a many-to-one > > > association, the cache key will contain the many-to-one associated > > entity. > > > If the associated entity is not enhanced, then it could be an > > uninitialized > > > proxy. > > > > > > I've created a test case [1] that illustrates this using Infinispan. > The > > > test case is for 5.1 branch, since hibernate-infinispan is still > included > > > in that branch. The same would happen for master as well. > > > > > > Aside from the obvious issue with increased memory requirements to > store > > an > > > entity, there are other problems as well. > > > > > > What I've found so far is that caching a key with an uninitialized > entity > > > proxy can cause some big problems: > > > > > > 1) A lookup will never find this key unless the lookup is done with a > > cache > > > key containing the same entity proxy instance. > > > > > > 2) Calling EntityManager#find with a composite ID containing a detached > > > uninitialized entity proxy will result in a > LazyInitializationException. > > > This does not happen with second-level cache disabled. > > > > > > 3) Calling EntityManager#find with a composite ID containing a managed > > > uninitialized entity proxy will result in the proxy being initialized. > > This > > > does not happen with second-level cache disabled. > > > > > > I have not looked into what happens when the associated entity is > > enhanced > > > and uninitialized (i.e., enhancement-as-proxy). > > > > > > IIUC, disassembling the ID that gets stored in the cache key would be > far > > > more efficient, and would avoid these issues. We would only want to do > > this > > > when a composite ID contains an association. This would require changes > > in > > > some SPIs though, to account for the disassembled ID type. > > > > > > I've been discussing necessary changes with Scott to cache a > > > disassembled ID. Before we get too far down this road, I'd like to get > > some > > > feedback. > > > > > > In the first place, should an entity instance be stored in a cache key? > > > > > > Is disassembling primary keys that contain an association the > appropriate > > > way to go? If so, I'll continue with a POC for doing this. > > > > > > Thanks, > > > Gail > > > > > > [1] > > > > > > https://github.com/gbadner/hibernate-core/blob/753e36edf5137296d28b2a07cee3daffc16c6d1e/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/CacheKeysWithEagerEntityFactoryTest.java > > > _______________________________________________ > > > hibernate-dev mailing list > > > hibernate-dev@lists.jboss.org > > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > > > > > > > _______________________________________________ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
