From 06963e693797f82cbc575e04436bd5ad6f0bf5f7 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Thu, 5 Jun 2025 22:56:09 +0200 Subject: [PATCH 1/4] HHH-19522 report failed optimistic lock checking for upsert() upsert() should throw StaleObjectStateException if the version verification fails --- .../sql/model/jdbc/MergeOperation.java | 3 +- .../sql/model/jdbc/UpsertOperation.java | 3 +- .../test/stateless/UpsertVersionedTest.java | 29 ++++++++++++++++++- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/MergeOperation.java b/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/MergeOperation.java index 2f98e88e0d5a..0e1bf7d0dd94 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/MergeOperation.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/MergeOperation.java @@ -23,7 +23,7 @@ public MergeOperation( MutationTarget mutationTarget, String sql, List parameterBinders) { - super( tableDetails, mutationTarget, sql, false, Expectation.None.INSTANCE, parameterBinders ); + super( tableDetails, mutationTarget, sql, false, new Expectation.RowCount(), parameterBinders ); } @Override @@ -31,5 +31,4 @@ public MutationType getMutationType() { return MutationType.UPDATE; } - } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/UpsertOperation.java b/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/UpsertOperation.java index a1a262c36240..6ba371d381c8 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/UpsertOperation.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/UpsertOperation.java @@ -23,7 +23,7 @@ public UpsertOperation( MutationTarget mutationTarget, String sql, List parameterBinders) { - super( tableDetails, mutationTarget, sql, false, Expectation.None.INSTANCE, parameterBinders ); + super( tableDetails, mutationTarget, sql, false, new Expectation.RowCount(), parameterBinders ); } @Override @@ -31,5 +31,4 @@ public MutationType getMutationType() { return MutationType.UPDATE; } - } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java index c08f714408c1..ad2dc1458801 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java @@ -7,17 +7,21 @@ import jakarta.persistence.Entity; import jakarta.persistence.Id; import jakarta.persistence.Version; +import org.hibernate.StaleObjectStateException; import org.hibernate.testing.orm.junit.DomainModel; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; import org.junit.jupiter.api.Test; -import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; @SessionFactory @DomainModel(annotatedClasses = UpsertVersionedTest.Record.class) public class UpsertVersionedTest { + @Test void test(SessionFactoryScope scope) { + scope.getSessionFactory().getSchemaManager().truncate(); scope.inStatelessTransaction(s-> { s.upsert(new Record(123L,null,"hello earth")); s.upsert(new Record(456L,2L,"hello mars")); @@ -41,6 +45,29 @@ public class UpsertVersionedTest { assertEquals("goodbye mars",s.get(Record.class,456L).message); }); } + + @Test void testStaleUpsert(SessionFactoryScope scope) { + scope.getSessionFactory().getSchemaManager().truncate(); + scope.inStatelessTransaction( s -> { + s.insert(new Record(789L, 1L, "hello world")); + } ); + scope.inStatelessTransaction( s -> { + s.upsert(new Record(789L, 1L, "hello mars")); + } ); + try { + scope.inStatelessTransaction( s -> { + s.upsert(new Record( 789L, 1L, "hello venus")); + } ); + fail(); + } + catch (StaleObjectStateException sose) { + //expected + } + scope.inStatelessTransaction( s-> { + assertEquals( "hello mars", s.get(Record.class,789L).message ); + } ); + } + @Entity(name = "Record") static class Record { @Id Long id; From c1d3a46be9bfa01f48bd72bdbc62be3056ac3d9e Mon Sep 17 00:00:00 2001 From: Gavin King Date: Thu, 5 Jun 2025 22:56:24 +0200 Subject: [PATCH 2/4] use @linkplain in jdoc --- .../src/main/java/org/hibernate/jdbc/Expectation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hibernate-core/src/main/java/org/hibernate/jdbc/Expectation.java b/hibernate-core/src/main/java/org/hibernate/jdbc/Expectation.java index bd595d58f0a6..941eb5e65d97 100644 --- a/hibernate-core/src/main/java/org/hibernate/jdbc/Expectation.java +++ b/hibernate-core/src/main/java/org/hibernate/jdbc/Expectation.java @@ -181,7 +181,7 @@ protected int expectedRowCount() { /** * Essentially identical to {@link RowCount} except that the row count - * is obtained via an output parameter of a {@link CallableStatement + * is obtained via an output parameter of a {@linkplain CallableStatement * stored procedure}. *

* Statement batching is disabled when {@code OutParameter} is used. From 05fcef64ddfd5ca4223fe356381a90cd7efa28b6 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Thu, 5 Jun 2025 23:35:04 +0200 Subject: [PATCH 3/4] HHH-19522 report failed optimistic lock checking for upsert() - also fix the problem for upsert emulation with UPDATE/INSERT --- .../jdbc/OptionalTableUpdateOperation.java | 33 ++++++++++++------- .../test/stateless/UpsertVersionedTest.java | 4 +-- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/OptionalTableUpdateOperation.java b/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/OptionalTableUpdateOperation.java index e34a5fbf339a..cc02e17755a3 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/OptionalTableUpdateOperation.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/OptionalTableUpdateOperation.java @@ -13,6 +13,7 @@ import java.util.Objects; import java.util.Set; +import org.hibernate.StaleStateException; import org.hibernate.engine.jdbc.mutation.JdbcValueBindings; import org.hibernate.engine.jdbc.mutation.ParameterUsage; import org.hibernate.engine.jdbc.mutation.group.PreparedStatementDetails; @@ -25,6 +26,7 @@ import org.hibernate.engine.jdbc.spi.MutationStatementPreparer; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; +import org.hibernate.exception.ConstraintViolationException; import org.hibernate.internal.util.collections.CollectionHelper; import org.hibernate.jdbc.Expectation; import org.hibernate.persister.entity.mutation.EntityMutationTarget; @@ -52,6 +54,7 @@ import org.hibernate.sql.model.internal.TableUpdateCustomSql; import org.hibernate.sql.model.internal.TableUpdateStandard; +import static org.hibernate.exception.ConstraintViolationException.ConstraintKind.UNIQUE; import static org.hibernate.sql.model.ModelMutationLogging.MODEL_MUTATION_LOGGER; /** @@ -152,20 +155,28 @@ public void performMutation( wasUpdated = false; } - if ( !wasUpdated ) { - MODEL_MUTATION_LOGGER.debugf( - "Upsert update altered no rows - inserting : %s", - tableMapping.getTableName() - ); - performInsert( jdbcValueBindings, session ); + if ( !wasUpdated ) { + MODEL_MUTATION_LOGGER.debugf( + "Upsert update altered no rows - inserting : %s", + tableMapping.getTableName() + ); + try { + performInsert( jdbcValueBindings, session ); + } + catch (ConstraintViolationException cve) { + throw cve.getKind() == UNIQUE + // assume it was the primary key constraint which was violated, + // due to a new version of the row existing in the database + ? new StaleStateException( mutationTarget.getRolePath(), cve ) + : cve; + } + } } } + finally { + jdbcValueBindings.afterStatement( tableMapping ); + } } - finally { - jdbcValueBindings.afterStatement( tableMapping ); - } - - } private void performDelete(JdbcValueBindings jdbcValueBindings, SharedSessionContractImplementor session) { final JdbcDeleteMutation jdbcDelete = createJdbcDelete( session ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java index ad2dc1458801..c6f21c49343a 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java @@ -7,7 +7,7 @@ import jakarta.persistence.Entity; import jakarta.persistence.Id; import jakarta.persistence.Version; -import org.hibernate.StaleObjectStateException; +import org.hibernate.StaleStateException; import org.hibernate.testing.orm.junit.DomainModel; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; @@ -60,7 +60,7 @@ public class UpsertVersionedTest { } ); fail(); } - catch (StaleObjectStateException sose) { + catch (StaleStateException sse) { //expected } scope.inStatelessTransaction( s-> { From 9e73a70add6ea5389eb25576b6d7f83f7b444229 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Fri, 6 Jun 2025 00:50:18 +0200 Subject: [PATCH 4/4] HHH-19522 report failed optimistic lock checking for upsert() - also fix the problem for Oracle --- .../model/jdbc/DeleteOrUpsertOperation.java | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/DeleteOrUpsertOperation.java b/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/DeleteOrUpsertOperation.java index 6af41093defa..5de3a1a086b7 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/DeleteOrUpsertOperation.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/DeleteOrUpsertOperation.java @@ -18,7 +18,9 @@ import org.hibernate.engine.jdbc.mutation.internal.PreparedStatementGroupSingleTable; import org.hibernate.engine.jdbc.mutation.spi.Binding; import org.hibernate.engine.jdbc.mutation.spi.BindingGroup; +import org.hibernate.engine.jdbc.spi.JdbcServices; import org.hibernate.engine.spi.SharedSessionContractImplementor; +import org.hibernate.jdbc.Expectation; import org.hibernate.persister.entity.mutation.EntityMutationTarget; import org.hibernate.persister.entity.mutation.EntityTableMapping; import org.hibernate.persister.entity.mutation.UpdateValuesAnalysis; @@ -44,6 +46,8 @@ public class DeleteOrUpsertOperation implements SelfExecutingUpdateOperation { private final OptionalTableUpdate optionalTableUpdate; + private final Expectation expectation = new Expectation.RowCount(); + public DeleteOrUpsertOperation( EntityMutationTarget mutationTarget, EntityTableMapping tableMapping, @@ -125,7 +129,8 @@ private void performDelete(JdbcValueBindings jdbcValueBindings, SharedSessionCon try { final PreparedStatement upsertDeleteStatement = statementDetails.resolveStatement(); - session.getJdbcServices().getSqlStatementLogger().logStatement( statementDetails.getSqlString() ); + final JdbcServices jdbcServices = session.getJdbcServices(); + jdbcServices.getSqlStatementLogger().logStatement( statementDetails.getSqlString() ); bindDeleteKeyValues( jdbcValueBindings, @@ -137,6 +142,16 @@ private void performDelete(JdbcValueBindings jdbcValueBindings, SharedSessionCon final int rowCount = session.getJdbcCoordinator().getResultSetReturn() .executeUpdate( upsertDeleteStatement, statementDetails.getSqlString() ); MODEL_MUTATION_LOGGER.tracef( "`%s` rows upsert-deleted from `%s`", rowCount, tableMapping.getTableName() ); + try { + expectation.verifyOutcome( rowCount, upsertDeleteStatement, -1, statementDetails.getSqlString() ); + } + catch (SQLException e) { + throw jdbcServices.getSqlExceptionHelper().convert( + e, + "Unable to verify outcome for upsert delete", + statementDetails.getSqlString() + ); + } } finally { statementDetails.releaseStatement( session ); @@ -204,14 +219,24 @@ private void performUpsert(JdbcValueBindings jdbcValueBindings, SharedSessionCon try { final PreparedStatement updateStatement = statementDetails.resolveStatement(); - session.getJdbcServices().getSqlStatementLogger().logStatement( statementDetails.getSqlString() ); - + final JdbcServices jdbcServices = session.getJdbcServices(); + jdbcServices.getSqlStatementLogger().logStatement( statementDetails.getSqlString() ); jdbcValueBindings.beforeStatement( statementDetails ); final int rowCount = session.getJdbcCoordinator().getResultSetReturn() .executeUpdate( updateStatement, statementDetails.getSqlString() ); MODEL_MUTATION_LOGGER.tracef( "`%s` rows upserted into `%s`", rowCount, tableMapping.getTableName() ); + try { + expectation.verifyOutcome( rowCount, updateStatement, -1, statementDetails.getSqlString() ); + } + catch (SQLException e) { + throw jdbcServices.getSqlExceptionHelper().convert( + e, + "Unable to verify outcome for upsert", + statementDetails.getSqlString() + ); + } } finally { statementDetails.releaseStatement( session );