HHH-18586 report StaleObjectStateExceptions when batch update fails

and some minor cleanups to the Coordinators

Signed-off-by: Gavin King <gavin@hibernate.org>

hibernate-core/src/main/java/org/hibernate/StaleObjectStateException.java

@@ -40,6 +40,19 @@ public class StaleObjectStateException extends StaleStateException {
 		this.identifier = identifier;
 	}
 
+	/**
+	 * Constructs a {@code StaleObjectStateException} using the supplied information
+	 * and cause.
+	 *
+	 * @param entityName The name of the entity
+	 * @param identifier The identifier of the entity
+	 */
+	public StaleObjectStateException(String entityName, Object identifier, StaleStateException cause) {
+		super( cause.getMessage(), cause );
+		this.entityName = entityName;
+		this.identifier = identifier;
+	}
+
 	public String getEntityName() {
 		return entityName;
 	}

hibernate-core/src/main/java/org/hibernate/StaleStateException.java

@@ -23,4 +23,15 @@ public class StaleStateException extends HibernateException {
 	public StaleStateException(String message) {
 		super( message );
 	}
+
+	/**
+	 * Constructs a {@code StaleStateException} using the supplied message
+	 * and cause.
+	 *
+	 * @param message The message explaining the exception condition
+	 * @param cause An exception to wrap
+	 */
+	public StaleStateException(String message, Exception cause) {
+		super( message, cause );
+	}
 }

hibernate-core/src/main/java/org/hibernate/engine/jdbc/batch/internal/BatchImpl.java

@@ -11,6 +11,7 @@ import java.sql.SQLException;
 import java.util.LinkedHashSet;
 
 import org.hibernate.HibernateException;
+import org.hibernate.StaleStateException;
 import org.hibernate.engine.jdbc.batch.spi.Batch;
 import org.hibernate.engine.jdbc.batch.spi.BatchKey;
 import org.hibernate.engine.jdbc.batch.spi.BatchObserver;
@@ -50,6 +51,7 @@ public class BatchImpl implements Batch {
 
 	private int batchPosition;
 	private boolean batchExecuted;
+	private StaleStateMapper[] staleStateMappers;
 
 	public BatchImpl(
 			BatchKey key,
@@ -97,6 +99,19 @@ public class BatchImpl implements Batch {
 		observers.add( observer );
 	}
 
+	@Override
+	public void addToBatch(
+			JdbcValueBindings jdbcValueBindings, TableInclusionChecker inclusionChecker,
+			StaleStateMapper staleStateMapper) {
+		if ( staleStateMapper != null ) {
+			if ( staleStateMappers == null ) {
+				staleStateMappers = new StaleStateMapper[batchSizeToUse];
+			}
+			staleStateMappers[batchPosition] = staleStateMapper;
+		}
+		addToBatch( jdbcValueBindings, inclusionChecker );
+	}
+
 	@Override
 	public void addToBatch(JdbcValueBindings jdbcValueBindings, TableInclusionChecker inclusionChecker) {
 		final boolean loggerTraceEnabled = BATCH_LOGGER.isTraceEnabled();
@@ -304,7 +319,8 @@ public class BatchImpl implements Batch {
 		}
 	}
 
-	private void checkRowCounts(int[] rowCounts, PreparedStatementDetails statementDetails) throws SQLException, HibernateException {
+	private void checkRowCounts(int[] rowCounts, PreparedStatementDetails statementDetails)
+			throws SQLException, HibernateException {
 		final int numberOfRowCounts = rowCounts.length;
 		if ( batchPosition != 0 ) {
 			if ( numberOfRowCounts != batchPosition ) {
@@ -317,7 +333,15 @@ public class BatchImpl implements Batch {
 		}
 
 		for ( int i = 0; i < numberOfRowCounts; i++ ) {
-			statementDetails.getExpectation().verifyOutcome( rowCounts[i], statementDetails.getStatement(), i, statementDetails.getSqlString() );
+			try {
+				statementDetails.getExpectation()
+						.verifyOutcome( rowCounts[i], statementDetails.getStatement(), i, statementDetails.getSqlString() );
+			}
+			catch ( StaleStateException staleStateException ) {
+				if ( staleStateMappers != null ) {
+					throw staleStateMappers[i].map( staleStateException );
+				}
+			}
 		}
 	}
 

hibernate-core/src/main/java/org/hibernate/engine/jdbc/batch/spi/Batch.java

@@ -9,7 +9,9 @@ package org.hibernate.engine.jdbc.batch.spi;
 import java.sql.PreparedStatement;
 import java.util.function.Supplier;
 
+import org.hibernate.HibernateException;
 import org.hibernate.Incubating;
+import org.hibernate.StaleStateException;
 import org.hibernate.engine.jdbc.mutation.JdbcValueBindings;
 import org.hibernate.engine.jdbc.mutation.TableInclusionChecker;
 import org.hibernate.engine.jdbc.mutation.group.PreparedStatementGroup;
@@ -51,6 +53,17 @@ public interface Batch {
 	 */
 	void addToBatch(JdbcValueBindings jdbcValueBindings, TableInclusionChecker inclusionChecker);
 
+	/**
+	 * Apply the value bindings to the batch JDBC statements and indicates completion
+	 * of the current part of the batch.
+	 */
+	void addToBatch(JdbcValueBindings jdbcValueBindings, TableInclusionChecker inclusionChecker, StaleStateMapper staleStateMapper);
+
+	@FunctionalInterface
+	interface StaleStateMapper {
+		HibernateException map(StaleStateException staleStateException);
+	}
+
 	/**
 	 * Execute this batch.
 	 */

hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/MutationExecutor.java

@@ -7,6 +7,7 @@
 package org.hibernate.engine.jdbc.mutation;
 
 import org.hibernate.Incubating;
+import org.hibernate.engine.jdbc.batch.spi.Batch;
 import org.hibernate.engine.jdbc.mutation.group.PreparedStatementDetails;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.generator.values.GeneratedValues;
@@ -50,5 +51,13 @@ public interface MutationExecutor {
 			OperationResultChecker resultChecker,
 			SharedSessionContractImplementor session);
 
+	GeneratedValues execute(
+			Object modelReference,
+			ValuesAnalysis valuesAnalysis,
+			TableInclusionChecker inclusionChecker,
+			OperationResultChecker resultChecker,
+			SharedSessionContractImplementor session,
+			Batch.StaleStateMapper staleStateMapper);
+
 	void release();
 }

hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/AbstractMutationExecutor.java

@@ -8,6 +8,7 @@ package org.hibernate.engine.jdbc.mutation.internal;
 
 import java.sql.SQLException;
 
+import org.hibernate.engine.jdbc.batch.spi.Batch;
 import org.hibernate.engine.jdbc.batch.spi.BatchKey;
 import org.hibernate.engine.jdbc.mutation.JdbcValueBindings;
 import org.hibernate.engine.jdbc.mutation.MutationExecutor;
@@ -21,6 +22,7 @@ import org.hibernate.persister.entity.mutation.EntityTableMapping;
 import org.hibernate.sql.model.TableMapping;
 import org.hibernate.sql.model.ValuesAnalysis;
 
+import static org.hibernate.engine.jdbc.mutation.internal.ModelMutationHelper.checkResults;
 import static org.hibernate.sql.model.ModelMutationLogging.MODEL_MUTATION_LOGGER;
 
 /**
@@ -52,6 +54,17 @@ public abstract class AbstractMutationExecutor implements MutationExecutor {
 			TableInclusionChecker inclusionChecker,
 			OperationResultChecker resultChecker,
 			SharedSessionContractImplementor session) {
+		return execute( modelReference, valuesAnalysis, inclusionChecker, resultChecker, session, null );
+	}
+
+	@Override
+	public final GeneratedValues execute(
+			Object modelReference,
+			ValuesAnalysis valuesAnalysis,
+			TableInclusionChecker inclusionChecker,
+			OperationResultChecker resultChecker,
+			SharedSessionContractImplementor session,
+			Batch.StaleStateMapper staleStateMapper) {
 		final GeneratedValues generatedValues = performNonBatchedOperations(
 				modelReference,
 				valuesAnalysis,
@@ -60,10 +73,12 @@ public abstract class AbstractMutationExecutor implements MutationExecutor {
 				session
 		);
 		performSelfExecutingOperations( valuesAnalysis, inclusionChecker, session );
-		performBatchedOperations( valuesAnalysis, inclusionChecker );
+		performBatchedOperations( valuesAnalysis, inclusionChecker, staleStateMapper );
 		return generatedValues;
 	}
 
+
+
 	protected GeneratedValues performNonBatchedOperations(
 			Object modelReference,
 			ValuesAnalysis valuesAnalysis,
@@ -81,7 +96,8 @@ public abstract class AbstractMutationExecutor implements MutationExecutor {
 
 	protected void performBatchedOperations(
 			ValuesAnalysis valuesAnalysis,
-			TableInclusionChecker inclusionChecker) {
+			TableInclusionChecker inclusionChecker,
+			Batch.StaleStateMapper staleStateMapper) {
 	}
 
 	/**
@@ -138,7 +154,7 @@ public abstract class AbstractMutationExecutor implements MutationExecutor {
 				return;
 			}
 
-			ModelMutationHelper.checkResults( resultChecker, statementDetails, affectedRowCount, -1 );
+			checkResults( resultChecker, statementDetails, affectedRowCount, -1 );
 		}
 		catch (SQLException e) {
 			throw session.getJdbcServices().getSqlExceptionHelper().convert(

hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/ModelMutationHelper.java

@@ -72,7 +72,7 @@ public class ModelMutationHelper {
 				if ( statistics.isStatisticsEnabled() ) {
 					statistics.optimisticFailure( mutationTarget.getNavigableRole().getFullPath() );
 				}
-				throw new StaleObjectStateException( mutationTarget.getNavigableRole().getFullPath(), id );
+				throw new StaleObjectStateException( mutationTarget.getNavigableRole().getFullPath(), id, e );
 			}
 			return false;
 		}

hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/MutationExecutorSingleBatched.java

@@ -56,8 +56,11 @@ public class MutationExecutorSingleBatched extends AbstractSingleMutationExecuto
 	}
 
 	@Override
-	protected void performBatchedOperations(ValuesAnalysis valuesAnalysis, TableInclusionChecker inclusionChecker) {
+	protected void performBatchedOperations(
-		resolveBatch().addToBatch( getJdbcValueBindings(), inclusionChecker );
+			ValuesAnalysis valuesAnalysis,
+			TableInclusionChecker inclusionChecker,
+			Batch.StaleStateMapper staleStateMapper) {
+		resolveBatch().addToBatch( getJdbcValueBindings(), inclusionChecker, staleStateMapper );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/MutationExecutorStandard.java

@@ -285,11 +285,12 @@ public class MutationExecutorStandard extends AbstractMutationExecutor implement
 	@Override
 	protected void performBatchedOperations(
 			ValuesAnalysis valuesAnalysis,
-			TableInclusionChecker inclusionChecker) {
+			TableInclusionChecker inclusionChecker,
+			Batch.StaleStateMapper staleStateMapper) {
 		if ( batch == null ) {
 			return;
 		}
-		batch.addToBatch( valueBindings, inclusionChecker );
+		batch.addToBatch( valueBindings, inclusionChecker, staleStateMapper );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/jdbc/Expectations.java

@@ -76,17 +76,15 @@ public class Expectations {
 			default:
 				if ( expectedRowCount > rowCount ) {
 					throw new StaleStateException(
-							"Batch update returned unexpected row count from update ["
+							"Batch update returned unexpected row count from update " + batchPosition
-									+ batchPosition + "]; actual row count: " + rowCount
+									+ actualVsExpected( expectedRowCount, rowCount )
-									+ "; expected: " + expectedRowCount + "; statement executed: "
+									+ " [" + sql + "]"
-									+ sql
 					);
 				}
 				else if ( expectedRowCount < rowCount ) {
 					throw new BatchedTooManyRowsAffectedException(
-							"Batch update returned unexpected row count from update [" +
+							"Batch update returned unexpected row count from update " + batchPosition
-							batchPosition + "]; actual row count: " + rowCount +
+									+ actualVsExpected( expectedRowCount, rowCount ),
-							"; expected: " + expectedRowCount,
 							expectedRowCount, rowCount, batchPosition );
 				}
 		}
@@ -95,18 +93,24 @@ public class Expectations {
 	static void checkNonBatched(int expectedRowCount, int rowCount, String sql) {
 		if ( expectedRowCount > rowCount ) {
 			throw new StaleStateException(
-					"Unexpected row count: " + rowCount + "; expected: " + expectedRowCount
+					"Unexpected row count"
-							+ "; statement executed: " + sql
+							+ actualVsExpected( expectedRowCount, rowCount )
+							+ " [" + sql + "]"
 			);
 		}
 		if ( expectedRowCount < rowCount ) {
 			throw new TooManyRowsAffectedException(
-					"Unexpected row count: " + rowCount + "; expected: " + expectedRowCount,
+					"Unexpected row count"
+							+ actualVsExpected( expectedRowCount, rowCount ),
 					1, rowCount
 			);
 		}
 	}
 
+	private static String actualVsExpected(int expectedRowCount, int rowCount) {
+		return " (expected row count " + expectedRowCount + " but was " + rowCount + ")";
+	}
+
 	// Various Expectation instances ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 	/**

hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java

@@ -2549,7 +2549,7 @@ public abstract class AbstractEntityPersister
 				if ( statistics.isStatisticsEnabled() ) {
 					statistics.optimisticFailure( getEntityName() );
 				}
-				throw new StaleObjectStateException( getEntityName(), id );
+				throw new StaleObjectStateException( getEntityName(), id, e );
 			}
 			return false;
 		}

hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/AbstractDeleteCoordinator.java

@@ -6,6 +6,8 @@
  */
 package org.hibernate.persister.entity.mutation;
 
+import org.hibernate.StaleObjectStateException;
+import org.hibernate.StaleStateException;
 import org.hibernate.engine.OptimisticLockStyle;
 import org.hibernate.engine.jdbc.batch.internal.BasicBatchKey;
 import org.hibernate.engine.jdbc.mutation.JdbcValueBindings;
@@ -93,7 +95,8 @@ public abstract class AbstractDeleteCoordinator
 			Object rowId,
 			Object[] loadedState,
 			SharedSessionContractImplementor session) {
-		final MutationOperationGroup operationGroup = generateOperationGroup( null, loadedState, true, session );
+		final MutationOperationGroup operationGroup =
+				generateOperationGroup( null, loadedState, true, session );
 		final MutationExecutor mutationExecutor = executor( session, operationGroup );
 
 		for ( int i = 0; i < operationGroup.getNumberOfOperations(); i++ ) {
@@ -118,15 +121,10 @@ public abstract class AbstractDeleteCoordinator
 					entity,
 					null,
 					null,
-					(statementDetails, affectedRowCount, batchPosition) -> identifiedResultsCheck(
+					(statementDetails, affectedRowCount, batchPosition) ->
-							statementDetails,
+							resultCheck( id, statementDetails, affectedRowCount, batchPosition ),
-							affectedRowCount,
+					session,
-							batchPosition,
+					staleStateException -> staleObjectState( id, staleStateException )
-							entityPersister(),
-							id,
-							factory()
-					),
-					session
 			);
 		}
 		finally {
@@ -171,31 +169,32 @@ public abstract class AbstractDeleteCoordinator
 			final EntityPersister persister = entityPersister();
 			final boolean[] versionability = persister.getPropertyVersionability();
 			for ( int attributeIndex = 0; attributeIndex < versionability.length; attributeIndex++ ) {
-				final AttributeMapping attribute;
 				// only makes sense to lock on singular attributes which are not excluded from optimistic locking
-				if ( versionability[attributeIndex] && !( attribute = persister.getAttributeMapping( attributeIndex ) ).isPluralAttributeMapping() ) {
+				if ( versionability[attributeIndex] ) {
-					final Object loadedValue = loadedState[attributeIndex];
+					final AttributeMapping attribute = persister.getAttributeMapping( attributeIndex );
-					if ( loadedValue != null ) {
+					if ( !attribute.isPluralAttributeMapping() ) {
-						final String mutationTableName = persister.getAttributeMutationTableName( attributeIndex );
+						final Object loadedValue = loadedState[attributeIndex];
-						attribute.breakDownJdbcValues(
+						if ( loadedValue != null ) {
-								loadedValue,
+							attribute.breakDownJdbcValues(
-								0,
+									loadedValue,
-								jdbcValueBindings,
+									0,
-								mutationTableName,
+									jdbcValueBindings,
-								(valueIndex, bindings, tableName, jdbcValue, jdbcValueMapping) -> {
+									persister.getAttributeMutationTableName( attributeIndex ),
-									if ( jdbcValue == null ) {
+									(valueIndex, bindings, tableName, jdbcValue, jdbcValueMapping) -> {
-										// presumably the SQL was generated with `is null`
+										if ( jdbcValue == null ) {
-										return;
+											// presumably the SQL was generated with `is null`
-									}
+											return;
-									bindings.bindValue(
+										}
-											jdbcValue,
+										bindings.bindValue(
-											tableName,
+												jdbcValue,
-											jdbcValueMapping.getSelectionExpression(),
+												tableName,
-											ParameterUsage.RESTRICT
+												jdbcValueMapping.getSelectionExpression(),
-									);
+												ParameterUsage.RESTRICT
-								},
+										);
-								session
+									},
-						);
+									session
+							);
+						}
 					}
 				}
 			}
@@ -230,7 +229,8 @@ public abstract class AbstractDeleteCoordinator
 			final MutationOperation jdbcMutation = operationGroup.getOperation( position );
 			final EntityTableMapping tableDetails = (EntityTableMapping) jdbcMutation.getTableDetails();
 			breakDownKeyJdbcValues( id, rowId, session, jdbcValueBindings, tableDetails );
-			final PreparedStatementDetails statementDetails = mutationExecutor.getPreparedStatementDetails( tableDetails.getTableName() );
+			final PreparedStatementDetails statementDetails =
+					mutationExecutor.getPreparedStatementDetails( tableDetails.getTableName() );
 			if ( statementDetails != null ) {
 				// force creation of the PreparedStatement
 				//noinspection resource
@@ -279,20 +279,31 @@ public abstract class AbstractDeleteCoordinator
 				entity,
 				null,
 				null,
-				(statementDetails, affectedRowCount, batchPosition) -> identifiedResultsCheck(
+				(statementDetails, affectedRowCount, batchPosition) ->
-						statementDetails,
+						resultCheck( id, statementDetails, affectedRowCount, batchPosition ),
-						affectedRowCount,
+				session,
-						batchPosition,
+				staleStateException -> staleObjectState( id, staleStateException )
-						entityPersister(),
-						id,
-						factory()
-				),
-				session
 		);
 
 		mutationExecutor.release();
 	}
 
+	private StaleObjectStateException staleObjectState(Object id, StaleStateException staleStateException) {
+		return new StaleObjectStateException( entityPersister.getEntityName(), id, staleStateException );
+	}
+
+	private boolean resultCheck(
+			Object id, PreparedStatementDetails statementDetails, int affectedRowCount, int batchPosition) {
+		return identifiedResultsCheck(
+				statementDetails,
+				affectedRowCount,
+				batchPosition,
+				entityPersister,
+				id,
+				factory
+		);
+	}
+
 	protected void applyStaticDeleteTableDetails(
 			Object id,
 			Object rowId,

hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/AbstractMutationCoordinator.java

@@ -95,7 +95,7 @@ public abstract class AbstractMutationCoordinator {
 				int outputIndex = 0;
 				int skipped = 0;
 				for ( int i = 0; i < mutationGroup.getNumberOfTableMutations(); i++ ) {
-					final TableMutation tableMutation = mutationGroup.getTableMutation( i );
+					final TableMutation<?> tableMutation = mutationGroup.getTableMutation( i );
 					final MutationOperation operation = tableMutation.createMutationOperation( valuesAnalysis, factory );
 					if ( operation != null ) {
 						operations[outputIndex++] = operation;

hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/InsertCoordinatorStandard.java

@@ -6,6 +6,7 @@
  */
 package org.hibernate.persister.entity.mutation;
 
+import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -17,6 +18,7 @@ import org.hibernate.engine.jdbc.mutation.JdbcValueBindings;
 import org.hibernate.engine.jdbc.mutation.MutationExecutor;
 import org.hibernate.engine.jdbc.mutation.ParameterUsage;
 import org.hibernate.engine.jdbc.mutation.TableInclusionChecker;
+import org.hibernate.engine.jdbc.mutation.group.PreparedStatementDetails;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.generator.BeforeExecutionGenerator;
@@ -192,15 +194,7 @@ public class InsertCoordinatorStandard extends AbstractMutationCoordinator imple
 					object,
 					insertValuesAnalysis,
 					tableInclusionChecker,
-					(statementDetails, affectedRowCount, batchPosition) -> {
+					InsertCoordinatorStandard::verifyOutcome,
-						statementDetails.getExpectation().verifyOutcome(
-								affectedRowCount,
-								statementDetails.getStatement(),
-								batchPosition,
-								statementDetails.getSqlString()
-						);
-						return true;
-					},
 					session
 			);
 		}
@@ -300,7 +294,8 @@ public class InsertCoordinatorStandard extends AbstractMutationCoordinator imple
 			SharedSessionContractImplementor session,
 			boolean forceIdentifierBinding) {
 		final boolean[] insertability = getPropertiesToInsert( values );
-		final MutationOperationGroup insertGroup = generateDynamicInsertSqlGroup( insertability, object, session, forceIdentifierBinding );
+		final MutationOperationGroup insertGroup =
+				generateDynamicInsertSqlGroup( insertability, object, session, forceIdentifierBinding );
 
 		final MutationExecutor mutationExecutor = executor( session, insertGroup, true );
 
@@ -315,15 +310,7 @@ public class InsertCoordinatorStandard extends AbstractMutationCoordinator imple
 					object,
 					insertValuesAnalysis,
 					tableInclusionChecker,
-					(statementDetails, affectedRowCount, batchPosition) -> {
+					InsertCoordinatorStandard::verifyOutcome,
-						statementDetails.getExpectation().verifyOutcome(
-								affectedRowCount,
-								statementDetails.getStatement(),
-								batchPosition,
-								statementDetails.getSqlString()
-						);
-						return true;
-					},
 					session
 			);
 		}
@@ -332,6 +319,17 @@ public class InsertCoordinatorStandard extends AbstractMutationCoordinator imple
 		}
 	}
 
+	private static boolean verifyOutcome(PreparedStatementDetails statementDetails, int affectedRowCount, int batchPosition)
+			throws SQLException {
+		statementDetails.getExpectation().verifyOutcome(
+				affectedRowCount,
+				statementDetails.getStatement(),
+				batchPosition,
+				statementDetails.getSqlString()
+		);
+		return true;
+	}
+
 	private MutationExecutor executor(SharedSessionContractImplementor session, MutationOperationGroup group, boolean dynamicUpdate) {
 		return mutationExecutorService
 				.createExecutor( resolveBatchKeyAccess( dynamicUpdate, session ), group, session );
@@ -415,7 +413,7 @@ public class InsertCoordinatorStandard extends AbstractMutationCoordinator imple
 							attributeInclusions[attributeIndex] = true;
 							attributeMapping.forEachInsertable( insertGroupBuilder );
 						}
-						else if ( isValueGenerationInSql( generator, factory().getJdbcServices().getDialect() ) ) {
+						else if ( isValueGenerationInSql( generator, factory.getJdbcServices().getDialect() ) ) {
 							handleValueGeneration( attributeMapping, insertGroupBuilder, (OnExecutionGenerator) generator );
 						}
 					}

hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java

@@ -14,6 +14,8 @@ import java.util.function.Supplier;
 import org.hibernate.AssertionFailure;
 import org.hibernate.HibernateException;
 import org.hibernate.Internal;
+import org.hibernate.StaleObjectStateException;
+import org.hibernate.StaleStateException;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.engine.OptimisticLockStyle;
 import org.hibernate.engine.jdbc.batch.internal.BasicBatchKey;
@@ -21,6 +23,7 @@ import org.hibernate.engine.jdbc.batch.spi.BatchKey;
 import org.hibernate.engine.jdbc.mutation.JdbcValueBindings;
 import org.hibernate.engine.jdbc.mutation.MutationExecutor;
 import org.hibernate.engine.jdbc.mutation.ParameterUsage;
+import org.hibernate.engine.jdbc.mutation.group.PreparedStatementDetails;
 import org.hibernate.engine.jdbc.mutation.internal.MutationQueryOptions;
 import org.hibernate.engine.jdbc.mutation.internal.NoBatchKeyAccess;
 import org.hibernate.engine.jdbc.mutation.spi.BatchKeyAccess;
@@ -99,6 +102,7 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 	}
 
 	//Used by Hibernate Reactive to efficiently create new instances of this same class
+	@SuppressWarnings("unused")
 	protected UpdateCoordinatorStandard(
 			EntityPersister entityPersister,
 			SessionFactoryImplementor factory,
@@ -296,7 +300,6 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 				session
 		);
 
-		//noinspection StatementWithEmptyBody
 		if ( valuesAnalysis.tablesNeedingUpdate.isEmpty() && valuesAnalysis.tablesNeedingDynamicUpdate.isEmpty() ) {
 			// nothing to do
 			return null;
@@ -477,9 +480,11 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 			SharedSessionContractImplementor session) {
 		assert versionUpdateGroup != null;
 
-		final EntityTableMapping mutatingTableDetails = (EntityTableMapping) versionUpdateGroup.getSingleOperation().getTableDetails();
+		final EntityTableMapping mutatingTableDetails =
+				(EntityTableMapping) versionUpdateGroup.getSingleOperation().getTableDetails();
 
-		final MutationExecutor mutationExecutor = updateVersionExecutor( session, versionUpdateGroup, false, batching );
+		final MutationExecutor mutationExecutor =
+				updateVersionExecutor( session, versionUpdateGroup, false, batching );
 
 		final EntityVersionMapping versionMapping = entityPersister().getVersionMapping();
 
@@ -494,14 +499,13 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 		// restrict the key
 		mutatingTableDetails.getKeyMapping().breakDownKeyJdbcValues(
 				id,
-				(jdbcValue, columnMapping) -> {
+				(jdbcValue, columnMapping) ->
-					mutationExecutor.getJdbcValueBindings().bindValue(
+						mutationExecutor.getJdbcValueBindings().bindValue(
-							jdbcValue,
+								jdbcValue,
-							mutatingTableDetails.getTableName(),
+								mutatingTableDetails.getTableName(),
-							columnMapping.getColumnName(),
+								columnMapping.getColumnName(),
-							ParameterUsage.RESTRICT
+								ParameterUsage.RESTRICT
-					);
+						),
-				},
 				session
 		);
 
@@ -517,16 +521,11 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 			return mutationExecutor.execute(
 					entity,
 					null,
-					(tableMapping) -> tableMapping.getTableName().equals( entityPersister().getIdentifierTableName() ),
+					tableMapping -> tableMapping.getTableName().equals( entityPersister.getIdentifierTableName() ),
-					(statementDetails, affectedRowCount, batchPosition) -> identifiedResultsCheck(
+					(statementDetails, affectedRowCount, batchPosition) ->
-							statementDetails,
+							resultCheck( id, statementDetails, affectedRowCount, batchPosition ),
-							affectedRowCount,
+					session,
-							batchPosition,
+					staleStateException -> staleObjectStateException( id, staleStateException )
-							entityPersister(),
-							id,
-							factory()
-					),
-					session
 			);
 		}
 		finally {
@@ -636,8 +635,7 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 
 			try {
 				if ( attributeMapping.getJdbcTypeCount() > 0
-						&& attributeMapping instanceof SingularAttributeMapping ) {
+						&& attributeMapping instanceof SingularAttributeMapping asSingularAttributeMapping ) {
-					SingularAttributeMapping asSingularAttributeMapping = (SingularAttributeMapping) attributeMapping;
 					processAttribute(
 							entity,
 							analysis,
@@ -771,15 +769,10 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 					entity,
 					valuesAnalysis,
 					valuesAnalysis.tablesNeedingUpdate::contains,
-					(statementDetails, affectedRowCount, batchPosition) -> identifiedResultsCheck(
+					(statementDetails, affectedRowCount, batchPosition) ->
-							statementDetails,
+							resultCheck( id, statementDetails, affectedRowCount, batchPosition ),
-							affectedRowCount,
+					session,
-							batchPosition,
+					staleStateException -> staleObjectStateException( id, staleStateException )
-							entityPersister(),
-							id,
-							factory()
-					),
-					session
 			);
 		}
 		finally {
@@ -911,7 +904,8 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 			return true;
 		}
 		else if ( entityPersister().getEntityMetamodel().isDynamicUpdate() && dirtinessChecker != null ) {
-			return attributeAnalysis.includeInSet() && dirtinessChecker.isDirty( attributeIndex, attributeMapping ).isDirty();
+			return attributeAnalysis.includeInSet()
+				&& dirtinessChecker.isDirty( attributeIndex, attributeMapping ).isDirty();
 		}
 		else {
 			return true;
@@ -948,7 +942,10 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 				valuesAnalysis,
 				mutationExecutor,
 				dynamicUpdateGroup,
-				(attributeIndex, attribute) -> dirtinessChecker.include( attributeIndex, (SingularAttributeMapping) attribute ) ? AttributeAnalysis.DirtynessStatus.CONSIDER_LIKE_DIRTY : AttributeAnalysis.DirtynessStatus.NOT_DIRTY,
+				(attributeIndex, attribute) ->
+						dirtinessChecker.include( attributeIndex, (SingularAttributeMapping) attribute )
+								? AttributeAnalysis.DirtynessStatus.CONSIDER_LIKE_DIRTY
+								: AttributeAnalysis.DirtynessStatus.NOT_DIRTY,
 				session
 		);
 		bindPartitionColumnValueBindings( oldValues, session, mutationExecutor.getJdbcValueBindings() );
@@ -957,28 +954,15 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 			return mutationExecutor.execute(
 					entity,
 					valuesAnalysis,
-					(tableMapping) -> {
+					tableMapping ->
-						if ( tableMapping.isOptional()
+							tableMapping.isOptional() && !valuesAnalysis.tablesWithNonNullValues.contains( tableMapping )
-								&& !valuesAnalysis.tablesWithNonNullValues.contains( tableMapping ) ) {
+									// the table is optional, and we have null values for all of its columns
-							// the table is optional, and we have null values for all of its columns
+									? valuesAnalysis.dirtyAttributeIndexes.length > 0
-							if ( valuesAnalysis.dirtyAttributeIndexes.length > 0 ) {
+									: valuesAnalysis.tablesNeedingUpdate.contains( tableMapping ),
-								return true;
+					(statementDetails, affectedRowCount, batchPosition) ->
-							}
+							resultCheck( id, statementDetails, affectedRowCount, batchPosition ),
-							return false;
+					session,
-						}
+					staleStateException -> staleObjectStateException( id, staleStateException )
-						else {
-							return valuesAnalysis.tablesNeedingUpdate.contains( tableMapping );
-						}
-					},
-					(statementDetails, affectedRowCount, batchPosition) -> identifiedResultsCheck(
-							statementDetails,
-							affectedRowCount,
-							batchPosition,
-							entityPersister(),
-							id,
-							factory()
-					),
-					session
 			);
 		}
 		finally {
@@ -986,12 +970,14 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 		}
 	}
 
-	private MutationExecutor executor(SharedSessionContractImplementor session, MutationOperationGroup group, boolean dynamicUpdate) {
+	private MutationExecutor executor(
+			SharedSessionContractImplementor session, MutationOperationGroup group, boolean dynamicUpdate) {
 		return mutationExecutorService
 				.createExecutor( resolveBatchKeyAccess( dynamicUpdate, session ), group, session );
 	}
 
-	private MutationExecutor updateVersionExecutor(SharedSessionContractImplementor session, MutationOperationGroup group, boolean dynamicUpdate) {
+	private MutationExecutor updateVersionExecutor(
+			SharedSessionContractImplementor session, MutationOperationGroup group, boolean dynamicUpdate) {
 		return mutationExecutorService
 				.createExecutor( resolveUpdateVersionBatchKeyAccess( dynamicUpdate, session ), group, session );
 	}
@@ -1014,8 +1000,9 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 				&& session.getTransactionCoordinator().isTransactionActive() ) {
 			return this::getVersionUpdateBatchkey;
 		}
-
+		else {
-		return NoBatchKeyAccess.INSTANCE;
+			return NoBatchKeyAccess.INSTANCE;
+		}
 	}
 
 	//Used by Hibernate Reactive
@@ -1023,6 +1010,22 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 		return versionUpdateBatchkey;
 	}
 
+	private boolean resultCheck(
+			Object id, PreparedStatementDetails statementDetails, int affectedRowCount, int batchPosition) {
+		return identifiedResultsCheck(
+				statementDetails,
+				affectedRowCount,
+				batchPosition,
+				entityPersister,
+				id,
+				factory
+		);
+	}
+
+	private StaleObjectStateException staleObjectStateException(Object id, StaleStateException staleStateException) {
+		return new StaleObjectStateException( entityPersister.getEntityName(), id, staleStateException );
+	}
+
 	protected MutationOperationGroup generateDynamicUpdateGroup(
 			Object entity,
 			Object id,
@@ -1035,7 +1038,6 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 		entityPersister().forEachMutableTable( (tableMapping) -> {
 			final MutatingTableReference tableReference = new MutatingTableReference( tableMapping );
 			final TableMutationBuilder<?> tableUpdateBuilder;
-			//noinspection SuspiciousMethodCalls
 			if ( ! valuesAnalysis.tablesNeedingUpdate.contains( tableReference.getTableMapping() ) ) {
 				// this table does not need updating
 				tableUpdateBuilder = new TableUpdateBuilderSkipped( tableReference );
@@ -1060,15 +1062,13 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 	}
 
 	private TableMutationBuilder<?> createTableUpdateBuilder(EntityTableMapping tableMapping) {
-		final GeneratedValuesMutationDelegate delegate = tableMapping.isIdentifierTable() ?
+		final GeneratedValuesMutationDelegate delegate =
-				entityPersister().getUpdateDelegate() :
+				tableMapping.isIdentifierTable()
-				null;
+						? entityPersister().getUpdateDelegate()
-		if ( delegate != null ) {
+						: null;
-			return delegate.createTableMutationBuilder( tableMapping.getInsertExpectation(), factory() );
+		return delegate != null
-		}
+				? delegate.createTableMutationBuilder( tableMapping.getInsertExpectation(), factory() )
-		else {
+				: newTableUpdateBuilder( tableMapping );
-			return newTableUpdateBuilder( tableMapping );
-		}
 	}
 
 	protected <O extends MutationOperation> AbstractTableUpdateBuilder<O> newTableUpdateBuilder(EntityTableMapping tableMapping) {
@@ -1099,7 +1099,8 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple
 				final AttributeAnalysis attributeAnalysis = updateValuesAnalysis.attributeAnalyses.get( attributeIndex );
 
 				if ( attributeAnalysis.includeInSet() ) {
-					assert updateValuesAnalysis.tablesNeedingUpdate.contains( tableMapping ) || updateValuesAnalysis.tablesNeedingDynamicUpdate.contains( tableMapping );
+					assert updateValuesAnalysis.tablesNeedingUpdate.contains( tableMapping )
+							|| updateValuesAnalysis.tablesNeedingDynamicUpdate.contains( tableMapping );
 					applyAttributeUpdateDetails(
 							entity,
 							updateGroupBuilder,

hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchOptimisticLockingTest.java

@@ -12,10 +12,8 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 
-import org.hibernate.StaleObjectStateException;
 import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.dialect.CockroachDialect;
-import org.hibernate.dialect.OracleDialect;
 
 import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
 import org.junit.Test;
@@ -109,9 +107,9 @@ public class BatchOptimisticLockingTest extends
 				);
 			}
 			else {
-				assertEquals(
+				assertTrue(
-						"Batch update returned unexpected row count from update [1]; actual row count: 0; expected: 1; statement executed: update Person set name=?,version=? where id=? and version=?",
 						expected.getMessage()
+								.startsWith("Batch update returned unexpected row count from update 1 (expected row count 1 but was 0) [update Person set name=?,version=? where id=? and version=?]")
 				);
 			}
 		}

hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchUpdateAndVersionTest.java

@@ -4,6 +4,8 @@ package org.hibernate.orm.test.batch;
 import java.util.ArrayList;
 import java.util.List;
 
+import jakarta.persistence.OptimisticLockException;
+import org.hibernate.StaleObjectStateException;
 import org.hibernate.cfg.AvailableSettings;
 
 import org.hibernate.testing.TestForIssue;
@@ -24,6 +26,8 @@ import jakarta.persistence.Table;
 import jakarta.persistence.Version;
 
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 @DomainModel(
 		annotatedClasses = {
@@ -34,17 +38,16 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 @ServiceRegistry(
 		settings = @Setting(name = AvailableSettings.STATEMENT_BATCH_SIZE, value = "2")
 )
-@TestForIssue(jiraKey = "HHH-16394")
 public class BatchUpdateAndVersionTest {
 
 	@Test
+	@TestForIssue(jiraKey = "HHH-16394")
 	public void testUpdate(SessionFactoryScope scope) {
+		scope.getSessionFactory().getSchemaManager().truncate();
 		scope.inTransaction(
 				session -> {
 					EntityA entityA1 = new EntityA( 1 );
-
 					EntityA entityA2 = new EntityA( 2 );
-
 					EntityA ownerA2 = new EntityA( 3 );
 
 					session.persist( ownerA2 );
@@ -80,6 +83,57 @@ public class BatchUpdateAndVersionTest {
 		);
 	}
 
+	@Test
+	public void testFailedUpdate(SessionFactoryScope scope) {
+		scope.getSessionFactory().getSchemaManager().truncate();
+		scope.inTransaction(
+				session -> {
+					EntityA entityA1 = new EntityA( 1 );
+					EntityA entityA2 = new EntityA( 2 );
+					EntityA ownerA2 = new EntityA( 3 );
+					session.persist( ownerA2 );
+					session.persist( entityA1 );
+					session.persist( entityA2 );
+				}
+		);
+
+		try {
+			scope.inTransaction(
+					session1 -> {
+						EntityA entityA1_1 = session1.get( EntityA.class, 1 );
+						assertThat( entityA1_1.getVersion() ).isEqualTo( 0 );
+						assertThat( entityA1_1.getPropertyA() ).isEqualTo( 0 );
+
+						EntityA entityA2_1 = session1.get( EntityA.class, 2 );
+						assertThat( entityA2_1.getVersion() ).isEqualTo( 0 );
+						assertThat( entityA2_1.getPropertyA() ).isEqualTo( 0 );
+
+						scope.inTransaction(
+								session2 -> {
+									EntityA entityA1_2 = session2.get( EntityA.class, 1 );
+									assertThat( entityA1_2.getVersion() ).isEqualTo( 0 );
+									assertThat( entityA1_2.getPropertyA() ).isEqualTo( 0 );
+
+									EntityA entityA2_2 = session2.get( EntityA.class, 2 );
+									assertThat( entityA2_2.getVersion() ).isEqualTo( 0 );
+									assertThat( entityA2_2.getPropertyA() ).isEqualTo( 0 );
+
+									entityA1_2.setPropertyA( 5 );
+									entityA2_2.setPropertyA( 5 );
+								}
+						);
+
+						entityA1_1.setPropertyA( 3 );
+						entityA2_1.setPropertyA( 3 );
+					}
+			);
+			fail();
+		}
+		catch (OptimisticLockException ole) {
+			assertTrue( ole.getCause() instanceof StaleObjectStateException );
+		}
+	}
+
 	@Entity(name = "EntityA")
 	@Table(name = "ENTITY_A")
 	public static class EntityA {

hibernate-core/src/test/java/org/hibernate/orm/test/jpa/transaction/batch/AbstractBatchingTest.java

@@ -115,6 +115,11 @@ public abstract class AbstractBatchingTest {
 			return wrapped.getStatementGroup();
 		}
 
+		@Override
+		public void addToBatch(JdbcValueBindings jdbcValueBindings, TableInclusionChecker inclusionChecker, StaleStateMapper staleStateMapper) {
+			addToBatch( jdbcValueBindings, inclusionChecker );
+		}
+
 		@Override
 		public void addToBatch(JdbcValueBindings jdbcValueBindings, TableInclusionChecker inclusionChecker) {
 			numberOfBatches++;