From 4d187565fe0723ee3830ab7fefb78d1413219d50 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Tue, 18 Nov 2025 08:55:12 +0100 Subject: [PATCH 1/6] HHH-19937 eliminate duplicate version check after refresh --- .../internal/DefaultRefreshEventListener.java | 66 ++++++++++--------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java index 0658a0899e82..1b974e5f8140 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java @@ -242,50 +242,52 @@ private static Object doRefresh( LazyInitializer lazyInitializer, Object id, PersistenceContext persistenceContext) { - // Handle the requested lock-mode (if one) in relation to the entry's (if one) current lock-mode + // Handle the requested lock mode, if any, in relation to the current + // lock mode, if any, of the entry. var lockOptionsToUse = event.getLockOptions(); final LockMode requestedLockMode = lockOptionsToUse.getLockMode(); final LockMode postRefreshLockMode; - if ( entry != null ) { + if ( entry == null ) { + // Should never happen now that we can't refresh detached entities. + postRefreshLockMode = null; + } + else { final LockMode currentLockMode = entry.getLockMode(); - if ( currentLockMode.greaterThan( requestedLockMode ) ) { - // the requested lock-mode is less restrictive than the current one - // - pass along the current lock-mode (after accounting for WRITE) - lockOptionsToUse = lockOptionsToUse.makeCopy(); - if ( currentLockMode == LockMode.WRITE - || currentLockMode == LockMode.PESSIMISTIC_WRITE - || currentLockMode == LockMode.PESSIMISTIC_READ ) { - // our transaction should already hold the exclusive lock on - // the underlying row - so READ should be sufficient. - // - // in fact, this really holds true for any current lock-mode that indicates we - // hold an exclusive lock on the underlying row - but we *need* to handle - // WRITE specially because the Loader/Locker mechanism does not allow for WRITE - // locks - lockOptionsToUse.setLockMode( LockMode.READ ); - // and prepare to reset the entry lock-mode to the previous lock mode after - // the refresh completes - postRefreshLockMode = currentLockMode; - } - else { - lockOptionsToUse.setLockMode( currentLockMode ); - postRefreshLockMode = null; - } + if ( requestedLockMode.greaterThan( currentLockMode ) + || currentLockMode == LockMode.NONE + || currentLockMode == LockMode.READ ) { + // Either the current transaction does not hold an exclusive + // lock or we're upgrading to a more restrictive lock mode. + // Nothing special to do in this case. + postRefreshLockMode = null; } else { - postRefreshLockMode = null; + // The requested lock mode is no more restrictive than the + // exclusive lock already held. Preserve the current mode. + lockOptionsToUse = lockOptionsToUse.makeCopy(); + // The current transaction already holds an exclusive lock, + // so refreshing with READ is sufficient. Also see HHH-19937; + // we want to avoid dupe version checks. + lockOptionsToUse.setLockMode( LockMode.READ ); + // But prepare to reset the entry lock mode to the previous + // lock mode after the refresh completes, except in the case + // where the two lock modes belong to the same level, in which + // case we will need to reset the entry to the requested mode. + postRefreshLockMode = + requestedLockMode.lessThan( currentLockMode ) + ? currentLockMode + : requestedLockMode; } } - else { - postRefreshLockMode = null; - } + // Go ahead and reload the entity from the database. final Object result = persister.load( id, object, lockOptionsToUse, source ); if ( result != null ) { - // apply postRefreshLockMode, if needed + // Apply the preserved postRefreshLockMode, if any. if ( postRefreshLockMode != null ) { - // if we get here, there was a previous entry, and we need to reset its lock mode - // - however, the refresh operation actually creates a new entry, so get it + // If we get here, there was a previous entry, and a lock was already held. + // But the refresh operation created a new entry with a less restrictive + // lock mode recorded. So we need to reset the lock mode of the new entry. persistenceContext.getEntry( result ).setLockMode( postRefreshLockMode ); } source.setReadOnly( result, isReadOnly( entry, persister, lazyInitializer, source ) ); From 879330154c3b9986dc81510b860552d2a7007d46 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Tue, 18 Nov 2025 09:05:00 +0100 Subject: [PATCH 2/6] use final vars in DefaultRefreshEventListener --- .../event/internal/DefaultRefreshEventListener.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java index 1b974e5f8140..a57a89eb2813 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java @@ -6,6 +6,7 @@ import org.hibernate.HibernateException; import org.hibernate.LockMode; +import org.hibernate.LockOptions; import org.hibernate.NonUniqueObjectException; import org.hibernate.TransientObjectException; import org.hibernate.UnresolvableObjectException; @@ -244,27 +245,30 @@ private static Object doRefresh( PersistenceContext persistenceContext) { // Handle the requested lock mode, if any, in relation to the current // lock mode, if any, of the entry. - var lockOptionsToUse = event.getLockOptions(); - final LockMode requestedLockMode = lockOptionsToUse.getLockMode(); + final var lockOptions = event.getLockOptions(); + final var requestedLockMode = lockOptions.getLockMode(); + final LockOptions lockOptionsToUse; final LockMode postRefreshLockMode; if ( entry == null ) { // Should never happen now that we can't refresh detached entities. + lockOptionsToUse = lockOptions; postRefreshLockMode = null; } else { - final LockMode currentLockMode = entry.getLockMode(); + final var currentLockMode = entry.getLockMode(); if ( requestedLockMode.greaterThan( currentLockMode ) || currentLockMode == LockMode.NONE || currentLockMode == LockMode.READ ) { // Either the current transaction does not hold an exclusive // lock or we're upgrading to a more restrictive lock mode. // Nothing special to do in this case. + lockOptionsToUse = lockOptions; postRefreshLockMode = null; } else { // The requested lock mode is no more restrictive than the // exclusive lock already held. Preserve the current mode. - lockOptionsToUse = lockOptionsToUse.makeCopy(); + lockOptionsToUse = lockOptions.makeCopy(); // The current transaction already holds an exclusive lock, // so refreshing with READ is sufficient. Also see HHH-19937; // we want to avoid dupe version checks. From f1803b0b341f662c6d3b808c4651a5ad2a7b450d Mon Sep 17 00:00:00 2001 From: Gavin King Date: Tue, 18 Nov 2025 11:19:20 +0100 Subject: [PATCH 3/6] HHH-19939 fix caching of MERGE and REFRESH load plans --- .../SingleIdEntityLoaderStandardImpl.java | 70 ++++++++++++------- 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleIdEntityLoaderStandardImpl.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleIdEntityLoaderStandardImpl.java index dd27177aeeb9..48e6db7f6c12 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleIdEntityLoaderStandardImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleIdEntityLoaderStandardImpl.java @@ -11,7 +11,6 @@ import org.hibernate.LockMode; import org.hibernate.LockOptions; import org.hibernate.engine.spi.LoadQueryInfluencers; -import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.loader.ast.spi.CascadingFetchProfile; import org.hibernate.metamodel.mapping.EntityMappingType; @@ -24,8 +23,10 @@ */ public class SingleIdEntityLoaderStandardImpl extends SingleIdEntityLoaderSupport { - private final EnumMap> selectByLockMode = new EnumMap<>( LockMode.class ); - private EnumMap> selectByInternalCascadeProfile; + private final EnumMap> selectByLockMode = + new EnumMap<>( LockMode.class ); + private final EnumMap>> selectByInternalCascadeProfile = + new EnumMap<>( CascadingFetchProfile.class ); private final BiFunction> loadPlanCreator; @@ -33,8 +34,7 @@ public SingleIdEntityLoaderStandardImpl( EntityMappingType entityDescriptor, LoadQueryInfluencers loadQueryInfluencers) { this( entityDescriptor, loadQueryInfluencers, - (lockOptions, influencers) -> - createLoadPlan( entityDescriptor, lockOptions, influencers, influencers.getSessionFactory() ) ); + (lockOptions, influencers) -> createLoadPlan( entityDescriptor, lockOptions, influencers) ); } /** @@ -50,13 +50,11 @@ protected SingleIdEntityLoaderStandardImpl( // todo (6.0) : consider creating a base AST and "cloning" it super( entityDescriptor, influencers.getSessionFactory() ); this.loadPlanCreator = loadPlanCreator; - // see org.hibernate.persister.entity.AbstractEntityPersister#createLoaders - // we should preload a few - maybe LockMode.NONE and LockMode.READ - final var noLocking = new LockOptions(); - final var singleIdLoadPlan = loadPlanCreator.apply( noLocking, influencers ); - if ( isLoadPlanReusable( noLocking, influencers ) ) { - selectByLockMode.put( LockMode.NONE, singleIdLoadPlan ); - } + // Preload some load plans (for now only do it for LockMode.NONE) + final var singleIdLoadPlan = loadPlanCreator.apply( LockOptions.NONE, influencers ); +// if ( isLoadPlanReusable( LockOptions.NONE, influencers ) ) { + selectByLockMode.put( LockMode.NONE, singleIdLoadPlan ); +// } } @Override @@ -114,38 +112,60 @@ private SingleIdLoadPlan getRegularLoadPlan(LockOptions lockOptions, LoadQuer } private SingleIdLoadPlan getInternalCascadeLoadPlan(LockOptions lockOptions, LoadQueryInfluencers influencers) { - final var fetchProfile = influencers.getEnabledCascadingFetchProfile(); - if ( selectByInternalCascadeProfile == null ) { - selectByInternalCascadeProfile = new EnumMap<>( CascadingFetchProfile.class ); + // TODO: It might be more efficient to just instantiate a LoadPlanKey + // object here than it is to maintain an EnumMap of EnumMaps + final var lockMode = lockOptions.getLockMode(); + EnumMap> map; + if ( isLoadPlanReusable( lockOptions, influencers ) ) { + final var fetchProfile = influencers.getEnabledCascadingFetchProfile(); + final var existingMap = selectByInternalCascadeProfile.get( fetchProfile ); + if ( existingMap == null ) { + map = new EnumMap<>( LockMode.class ); + selectByInternalCascadeProfile.put( fetchProfile, map ); + } + else { + final var existing = existingMap.get( lockMode ); + if ( existing != null ) { + return existing; + } + else { + map = existingMap; + } + } } else { - final var existing = selectByInternalCascadeProfile.get( fetchProfile ); - if ( existing != null ) { - return existing; - } + map = null; } + final var plan = loadPlanCreator.apply( lockOptions, influencers ); - selectByInternalCascadeProfile.put( fetchProfile, plan ); + if ( map != null ) { + map.put( lockMode, plan ); + } return plan; } + /** + * We key the caches only by {@link LockMode} and {@link CascadingFetchProfile}. + * If there is a pessimistic lock with non-default options like timeout, a custom + * fetch profile, or an entity graph, we don't cache and reuse the plan. + */ private boolean isLoadPlanReusable(LockOptions lockOptions, LoadQueryInfluencers influencers) { if ( lockOptions.getLockMode().isPessimistic() && lockOptions.hasNonDefaultOptions() ) { return false; } else { - return !getLoadable().isAffectedByEntityGraph( influencers ) - && !getLoadable().isAffectedByEnabledFetchProfiles( influencers ); + final var loadable = getLoadable(); + return !loadable.isAffectedByEntityGraph( influencers ) + && !loadable.isAffectedByEnabledFetchProfiles( influencers ); } } private static SingleIdLoadPlan createLoadPlan( EntityMappingType loadable, LockOptions lockOptions, - LoadQueryInfluencers influencers, - SessionFactoryImplementor factory) { - + LoadQueryInfluencers influencers) { final var jdbcParametersBuilder = JdbcParametersList.newBuilder(); + final var factory = influencers.getSessionFactory(); return new SingleIdLoadPlan<>( loadable, loadable.getIdentifierMapping(), From 2561595cfa8f8d9ffbefa8d9413c37e90547e2c7 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Tue, 18 Nov 2025 11:20:54 +0100 Subject: [PATCH 4/6] HHH-19939, HHH-19937 add tests --- .../org/hibernate/orm/test/locking/C.java | 60 ++++++++++++ .../orm/test/locking/LockModeTest.java | 91 ++++++++++++++++++- 2 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/locking/C.java diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/locking/C.java b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/C.java new file mode 100644 index 000000000000..3ed51ff3c278 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/C.java @@ -0,0 +1,60 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.locking; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.Table; +import jakarta.persistence.Version; +import org.hibernate.annotations.GenericGenerator; + +/** + * @author Steve Ebersole + */ +@Entity +@Table( name = "T_LOCK_C" ) +public class C { + private Long id; + private String value; + private int version; + + public C() { + } + + public C(String value) { + this.value = value; + } + + @Id + @GeneratedValue( generator = "increment" ) + @GenericGenerator( name = "increment", strategy = "increment" ) + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + @Column(name="c_value") + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } + + @Version + public int getVersion() { + return version; + } + + public void setVersion(int version) { + this.version = version; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockModeTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockModeTest.java index 43d3ff9919fd..d1fb1d5b1365 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockModeTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockModeTest.java @@ -14,6 +14,7 @@ import org.hibernate.LockMode; import org.hibernate.Session; +import org.hibernate.StaleObjectStateException; import org.hibernate.boot.registry.StandardServiceRegistryBuilder; import org.hibernate.cfg.AvailableSettings; import org.hibernate.community.dialect.AltibaseDialect; @@ -22,6 +23,7 @@ import org.hibernate.dialect.SQLServerDialect; import org.hibernate.dialect.SybaseASEDialect; import org.hibernate.dialect.SybaseDialect; +import org.hibernate.dialect.lock.OptimisticEntityLockException; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.query.criteria.HibernateCriteriaBuilder; import org.hibernate.query.criteria.JpaCriteriaQuery; @@ -43,6 +45,7 @@ import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; /** @@ -56,10 +59,11 @@ public class LockModeTest extends BaseSessionFactoryFunctionalTest { private Long id; + private Long cid; @Override protected Class[] getAnnotatedClasses() { - return new Class[] { A.class }; + return new Class[] { A.class, C.class }; } @Override @@ -79,6 +83,9 @@ public void prepareTest() throws Exception { A a = new A( "it" ); session.persist( a ); id = a.getId(); + C c = new C( "it" ); + session.persist( c ); + cid = c.getId(); } ); } @@ -263,6 +270,88 @@ public void testRefreshWithExplicitHigherLevelLockMode2() { } ); } + @Test + @JiraKey(value = "HHH-19937") + public void testRefreshWithOptimisticExplicitHigherLevelLockMode() { + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id ); + checkLockMode( c, LockMode.READ, session ); + session.refresh( c, LockModeType.OPTIMISTIC ); + checkLockMode( c, LockMode.OPTIMISTIC, session ); + session.refresh( c ); + checkLockMode( c, LockMode.OPTIMISTIC, session ); + } ); + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id ); + checkLockMode( c, LockMode.READ, session ); + session.refresh( c, LockModeType.OPTIMISTIC ); + checkLockMode( c, LockMode.OPTIMISTIC, session ); + session.refresh( c, LockMode.OPTIMISTIC_FORCE_INCREMENT ); + checkLockMode( c, LockMode.OPTIMISTIC_FORCE_INCREMENT, session ); + } ); + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id ); + checkLockMode( c, LockMode.READ, session ); + session.refresh( c, LockModeType.OPTIMISTIC_FORCE_INCREMENT ); + checkLockMode( c, LockMode.OPTIMISTIC_FORCE_INCREMENT, session ); + session.refresh( c ); + checkLockMode( c, LockMode.OPTIMISTIC_FORCE_INCREMENT, session ); + } ); + } + + @Test + @JiraKey(value = "HHH-19937") + public void testRefreshWithOptimisticLockRefresh() { + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id, LockModeType.OPTIMISTIC ); + doInHibernate( this::sessionFactory, s -> { + s.find( C.class, id ).setValue( "new value" ); + } ); + session.refresh( c ); + } ); + } + + @Test + @JiraKey(value = "HHH-19937") + public void testRefreshWithOptimisticForceIncrementLockRefresh() { + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id, LockModeType.OPTIMISTIC_FORCE_INCREMENT ); + doInHibernate( this::sessionFactory, s -> { + s.find( C.class, id ).setValue( "new value" ); + } ); + session.refresh( c ); + } ); + } + + @Test + @JiraKey(value = "HHH-19937") + public void testRefreshWithOptimisticLockFailure() { + assertThrows( OptimisticEntityLockException.class, () -> + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id, LockModeType.OPTIMISTIC ); + session.refresh( c ); + doInHibernate( this::sessionFactory, s -> { + s.find( C.class, id ).setValue( "new value" ); + } ); + } ) + ); + } + + @Test + @JiraKey(value = "HHH-19937") + public void testRefreshWithOptimisticForceIncrementLockFailure() { + // TODO: shouldn't this also be an OptimisticEntityLockException + assertThrows( StaleObjectStateException.class, () -> + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id, LockModeType.OPTIMISTIC_FORCE_INCREMENT ); + session.refresh( c ); + doInHibernate( this::sessionFactory, s -> { + s.find( C.class, id ).setValue( "new value" ); + } ); + } ) + ); + } + @Test @JiraKey(value = "HHH-12257") public void testRefreshAfterUpdate() { From 09859365339b1066201442172f956c506f9025d2 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Tue, 18 Nov 2025 11:21:14 +0100 Subject: [PATCH 5/6] minor code clanups --- .../main/java/org/hibernate/LockOptions.java | 8 ++--- .../loader/ast/internal/SingleIdLoadPlan.java | 31 +++++++++---------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/LockOptions.java b/hibernate-core/src/main/java/org/hibernate/LockOptions.java index 38579c52a6eb..572ea9f7c18e 100644 --- a/hibernate-core/src/main/java/org/hibernate/LockOptions.java +++ b/hibernate-core/src/main/java/org/hibernate/LockOptions.java @@ -308,8 +308,8 @@ public LockOptions setFollowOnStrategy(Locking.FollowOn followOnStrategy) { public boolean hasNonDefaultOptions() { return timeout != Timeouts.WAIT_FOREVER_MILLI - || scope != Locking.Scope.ROOT_ONLY - || followOnStrategy != Locking.FollowOn.ALLOW; + || scope != Locking.Scope.ROOT_ONLY + || followOnStrategy != Locking.FollowOn.ALLOW; } @@ -588,7 +588,7 @@ public LockMode findGreatestLockMode() { */ @Deprecated(since = "7", forRemoval = true) public LockOptions makeCopy() { - final LockOptions copy = new LockOptions(); + final var copy = new LockOptions(); copy( this, copy ); return copy; } @@ -606,7 +606,7 @@ public LockOptions makeDefensiveCopy() { return this; } else { - final LockOptions copy = new LockOptions(); + final var copy = new LockOptions(); copy( this, copy ); return copy; } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleIdLoadPlan.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleIdLoadPlan.java index d025073601d0..745a7921751f 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleIdLoadPlan.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleIdLoadPlan.java @@ -5,8 +5,6 @@ package org.hibernate.loader.ast.internal; import org.hibernate.LockOptions; -import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment; -import org.hibernate.engine.jdbc.spi.JdbcServices; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.loader.ast.spi.Loadable; @@ -15,7 +13,6 @@ import org.hibernate.query.internal.SimpleQueryOptions; import org.hibernate.query.spi.QueryOptions; import org.hibernate.query.spi.QueryOptionsAdapter; -import org.hibernate.sql.ast.SqlAstTranslatorFactory; import org.hibernate.sql.ast.tree.select.SelectStatement; import org.hibernate.sql.exec.internal.BaseExecutionContext; import org.hibernate.sql.exec.internal.CallbackImpl; @@ -54,21 +51,21 @@ public SingleIdLoadPlan( SessionFactoryImplementor sessionFactory) { this.entityMappingType = entityMappingType; this.restrictivePart = restrictivePart; - this.lockOptions = lockOptions.makeCopy(); + this.lockOptions = lockOptions.makeDefensiveCopy(); this.jdbcParameters = jdbcParameters; - final JdbcServices jdbcServices = sessionFactory.getJdbcServices(); - final JdbcEnvironment jdbcEnvironment = jdbcServices.getJdbcEnvironment(); - final SqlAstTranslatorFactory sqlAstTranslatorFactory = jdbcEnvironment.getSqlAstTranslatorFactory(); - this.jdbcSelect = sqlAstTranslatorFactory.buildSelectTranslator( sessionFactory, sqlAst ) - .translate( - null, - new QueryOptionsAdapter() { - @Override - public LockOptions getLockOptions() { - return lockOptions; - } - } - ); + this.jdbcSelect = + sessionFactory.getJdbcServices().getJdbcEnvironment() + .getSqlAstTranslatorFactory() + .buildSelectTranslator( sessionFactory, sqlAst ) + .translate( + null, + new QueryOptionsAdapter() { + @Override + public LockOptions getLockOptions() { + return lockOptions; + } + } + ); } protected LockOptions getLockOptions() { From b56fce8ac4b682abf7dd59f26a623ccfa7a1d72e Mon Sep 17 00:00:00 2001 From: Gavin King Date: Tue, 18 Nov 2025 12:02:59 +0100 Subject: [PATCH 6/6] HHH-19939, HHH-19937 fix tests on MySQL/Informix --- .../orm/test/locking/LockModeTest.java | 109 +------------ .../test/locking/OptimisticLockModeTest.java | 151 ++++++++++++++++++ 2 files changed, 158 insertions(+), 102 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/locking/OptimisticLockModeTest.java diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockModeTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockModeTest.java index d1fb1d5b1365..312a0ed98c8a 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockModeTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockModeTest.java @@ -4,48 +4,42 @@ */ package org.hibernate.orm.test.locking; -import java.sql.Connection; -import java.util.Collections; - import jakarta.persistence.LockModeType; import jakarta.persistence.Timeout; import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaQuery; - import org.hibernate.LockMode; import org.hibernate.Session; -import org.hibernate.StaleObjectStateException; import org.hibernate.boot.registry.StandardServiceRegistryBuilder; -import org.hibernate.cfg.AvailableSettings; import org.hibernate.community.dialect.AltibaseDialect; import org.hibernate.community.dialect.InformixDialect; import org.hibernate.dialect.CockroachDialect; import org.hibernate.dialect.SQLServerDialect; import org.hibernate.dialect.SybaseASEDialect; import org.hibernate.dialect.SybaseDialect; -import org.hibernate.dialect.lock.OptimisticEntityLockException; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.query.criteria.HibernateCriteriaBuilder; import org.hibernate.query.criteria.JpaCriteriaQuery; import org.hibernate.query.sqm.tree.domain.SqmPath; - import org.hibernate.testing.orm.junit.BaseSessionFactoryFunctionalTest; import org.hibernate.testing.orm.junit.DialectFeatureChecks; +import org.hibernate.testing.orm.junit.JiraKey; import org.hibernate.testing.orm.junit.RequiresDialectFeature; import org.hibernate.testing.orm.junit.SkipForDialect; -import org.hibernate.testing.orm.junit.JiraKey; import org.hibernate.testing.transaction.TransactionUtil; import org.hibernate.testing.util.ExceptionUtil; - import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.Collections; + import static jakarta.persistence.LockModeType.NONE; import static jakarta.persistence.LockModeType.PESSIMISTIC_WRITE; +import static java.sql.Connection.TRANSACTION_REPEATABLE_READ; +import static org.hibernate.cfg.JdbcSettings.ISOLATION; import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; /** @@ -59,21 +53,17 @@ public class LockModeTest extends BaseSessionFactoryFunctionalTest { private Long id; - private Long cid; @Override protected Class[] getAnnotatedClasses() { - return new Class[] { A.class, C.class }; + return new Class[] { A.class }; } @Override protected void applySettings(StandardServiceRegistryBuilder ssrBuilder) { super.applySettings( ssrBuilder ); - // We can't use a shared connection provider if we use T ransactionUtil.setJdbcTimeout because that is set on the connection level -// ssrBuilder.getSettings().remove( AvailableSettings.CONNECTION_PROVIDER ); if ( getDialect() instanceof InformixDialect ) { - ssrBuilder.applySetting( AvailableSettings.ISOLATION, - Connection.TRANSACTION_REPEATABLE_READ ); + ssrBuilder.applySetting( ISOLATION, TRANSACTION_REPEATABLE_READ ); } } @@ -83,9 +73,6 @@ public void prepareTest() throws Exception { A a = new A( "it" ); session.persist( a ); id = a.getId(); - C c = new C( "it" ); - session.persist( c ); - cid = c.getId(); } ); } @@ -270,88 +257,6 @@ public void testRefreshWithExplicitHigherLevelLockMode2() { } ); } - @Test - @JiraKey(value = "HHH-19937") - public void testRefreshWithOptimisticExplicitHigherLevelLockMode() { - doInHibernate( this::sessionFactory, session -> { - C c = session.find( C.class, id ); - checkLockMode( c, LockMode.READ, session ); - session.refresh( c, LockModeType.OPTIMISTIC ); - checkLockMode( c, LockMode.OPTIMISTIC, session ); - session.refresh( c ); - checkLockMode( c, LockMode.OPTIMISTIC, session ); - } ); - doInHibernate( this::sessionFactory, session -> { - C c = session.find( C.class, id ); - checkLockMode( c, LockMode.READ, session ); - session.refresh( c, LockModeType.OPTIMISTIC ); - checkLockMode( c, LockMode.OPTIMISTIC, session ); - session.refresh( c, LockMode.OPTIMISTIC_FORCE_INCREMENT ); - checkLockMode( c, LockMode.OPTIMISTIC_FORCE_INCREMENT, session ); - } ); - doInHibernate( this::sessionFactory, session -> { - C c = session.find( C.class, id ); - checkLockMode( c, LockMode.READ, session ); - session.refresh( c, LockModeType.OPTIMISTIC_FORCE_INCREMENT ); - checkLockMode( c, LockMode.OPTIMISTIC_FORCE_INCREMENT, session ); - session.refresh( c ); - checkLockMode( c, LockMode.OPTIMISTIC_FORCE_INCREMENT, session ); - } ); - } - - @Test - @JiraKey(value = "HHH-19937") - public void testRefreshWithOptimisticLockRefresh() { - doInHibernate( this::sessionFactory, session -> { - C c = session.find( C.class, id, LockModeType.OPTIMISTIC ); - doInHibernate( this::sessionFactory, s -> { - s.find( C.class, id ).setValue( "new value" ); - } ); - session.refresh( c ); - } ); - } - - @Test - @JiraKey(value = "HHH-19937") - public void testRefreshWithOptimisticForceIncrementLockRefresh() { - doInHibernate( this::sessionFactory, session -> { - C c = session.find( C.class, id, LockModeType.OPTIMISTIC_FORCE_INCREMENT ); - doInHibernate( this::sessionFactory, s -> { - s.find( C.class, id ).setValue( "new value" ); - } ); - session.refresh( c ); - } ); - } - - @Test - @JiraKey(value = "HHH-19937") - public void testRefreshWithOptimisticLockFailure() { - assertThrows( OptimisticEntityLockException.class, () -> - doInHibernate( this::sessionFactory, session -> { - C c = session.find( C.class, id, LockModeType.OPTIMISTIC ); - session.refresh( c ); - doInHibernate( this::sessionFactory, s -> { - s.find( C.class, id ).setValue( "new value" ); - } ); - } ) - ); - } - - @Test - @JiraKey(value = "HHH-19937") - public void testRefreshWithOptimisticForceIncrementLockFailure() { - // TODO: shouldn't this also be an OptimisticEntityLockException - assertThrows( StaleObjectStateException.class, () -> - doInHibernate( this::sessionFactory, session -> { - C c = session.find( C.class, id, LockModeType.OPTIMISTIC_FORCE_INCREMENT ); - session.refresh( c ); - doInHibernate( this::sessionFactory, s -> { - s.find( C.class, id ).setValue( "new value" ); - } ); - } ) - ); - } - @Test @JiraKey(value = "HHH-12257") public void testRefreshAfterUpdate() { diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/locking/OptimisticLockModeTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/OptimisticLockModeTest.java new file mode 100644 index 000000000000..d40f4d95e549 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/OptimisticLockModeTest.java @@ -0,0 +1,151 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.locking; + +import jakarta.persistence.LockModeType; +import org.hibernate.LockMode; +import org.hibernate.Session; +import org.hibernate.StaleObjectStateException; +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; +import org.hibernate.dialect.MySQLDialect; +import org.hibernate.dialect.lock.OptimisticEntityLockException; +import org.hibernate.engine.spi.SharedSessionContractImplementor; +import org.hibernate.testing.orm.junit.BaseSessionFactoryFunctionalTest; +import org.hibernate.testing.orm.junit.JiraKey; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + + +import static java.sql.Connection.TRANSACTION_READ_COMMITTED; +import static org.hibernate.cfg.JdbcSettings.ISOLATION; +import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * Make sure that directly specifying lock modes, even though deprecated, continues to work until removed. + * + * @author Steve Ebersole + */ +@JiraKey( value = "HHH-5275") +//@SkipForDialect(dialectClass = SybaseASEDialect.class, majorVersion = 15, +// reason = "skip this test on Sybase ASE 15.5, but run it on 15.7, see HHH-6820") +public class OptimisticLockModeTest extends BaseSessionFactoryFunctionalTest { + + private Long id; + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { C.class }; + } + + @Override + protected void applySettings(StandardServiceRegistryBuilder ssrBuilder) { + super.applySettings( ssrBuilder ); + if ( getDialect() instanceof MySQLDialect ) { + ssrBuilder.applySetting( ISOLATION, TRANSACTION_READ_COMMITTED ); + } + } + + @BeforeEach + public void prepareTest() throws Exception { + doInHibernate( this::sessionFactory, session -> { + C c = new C( "it" ); + session.persist( c ); + id = c.getId(); + } ); + } + + @Override + protected boolean isCleanupTestDataRequired(){return true;} + + @Test + @JiraKey(value = "HHH-19937") + public void testRefreshWithOptimisticLockRefresh() { + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id, LockModeType.OPTIMISTIC ); + doInHibernate( this::sessionFactory, s -> { + s.find( C.class, id ).setValue( "new value" ); + } ); + session.refresh( c ); + } ); + } + + @Test + @JiraKey(value = "HHH-19937") + public void testRefreshWithOptimisticForceIncrementLockRefresh() { + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id, LockModeType.OPTIMISTIC_FORCE_INCREMENT ); + doInHibernate( this::sessionFactory, s -> { + s.find( C.class, id ).setValue( "new value" ); + } ); + session.refresh( c ); + } ); + } + + @Test + @JiraKey(value = "HHH-19937") + public void testRefreshWithOptimisticLockFailure() { + assertThrows( OptimisticEntityLockException.class, () -> + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id, LockModeType.OPTIMISTIC ); + session.refresh( c ); + doInHibernate( this::sessionFactory, s -> { + s.find( C.class, id ).setValue( "new value" ); + } ); + } ) + ); + } + + @Test + @JiraKey(value = "HHH-19937") + public void testRefreshWithOptimisticForceIncrementLockFailure() { + // TODO: shouldn't this also be an OptimisticEntityLockException + assertThrows( StaleObjectStateException.class, () -> + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id, LockModeType.OPTIMISTIC_FORCE_INCREMENT ); + session.refresh( c ); + doInHibernate( this::sessionFactory, s -> { + s.find( C.class, id ).setValue( "new value" ); + } ); + } ) + ); + } + + @Test + @JiraKey(value = "HHH-19937") + public void testRefreshWithOptimisticExplicitHigherLevelLockMode() { + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id ); + checkLockMode( c, LockMode.READ, session ); + session.refresh( c, LockModeType.OPTIMISTIC ); + checkLockMode( c, LockMode.OPTIMISTIC, session ); + session.refresh( c ); + checkLockMode( c, LockMode.OPTIMISTIC, session ); + } ); + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id ); + checkLockMode( c, LockMode.READ, session ); + session.refresh( c, LockModeType.OPTIMISTIC ); + checkLockMode( c, LockMode.OPTIMISTIC, session ); + session.refresh( c, LockMode.OPTIMISTIC_FORCE_INCREMENT ); + checkLockMode( c, LockMode.OPTIMISTIC_FORCE_INCREMENT, session ); + } ); + doInHibernate( this::sessionFactory, session -> { + C c = session.find( C.class, id ); + checkLockMode( c, LockMode.READ, session ); + session.refresh( c, LockModeType.OPTIMISTIC_FORCE_INCREMENT ); + checkLockMode( c, LockMode.OPTIMISTIC_FORCE_INCREMENT, session ); + session.refresh( c ); + checkLockMode( c, LockMode.OPTIMISTIC_FORCE_INCREMENT, session ); + } ); + } + + private void checkLockMode(Object entity, LockMode expectedLockMode, Session session) { + final LockMode lockMode = + ( (SharedSessionContractImplementor) session ).getPersistenceContext().getEntry( entity ).getLockMode(); + assertEquals( expectedLockMode, lockMode ); + } +}