HHH-17688 Make statement release more consistent in mutation delegates

mbellade · beikov · commit 9f3676d690bd · 2024-02-07T10:16:02.000+01:00
diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/AbstractMutationExecutor.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/AbstractMutationExecutor.java
@@ -99,9 +99,10 @@ protected void performNonBatchedMutation(
 
 		// If we get here the statement is needed - make sure it is resolved
 		session.getJdbcServices().getSqlStatementLogger().logStatement( statementDetails.getSqlString() );
-		valueBindings.beforeStatement( statementDetails );
 
 		try {
+			valueBindings.beforeStatement( statementDetails );
+
 			final int affectedRowCount = session.getJdbcCoordinator()
 					.getResultSetReturn()
 					.executeUpdate( statementDetails.getStatement(), statementDetails.getSqlString() );
diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/MutationExecutorPostInsert.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/MutationExecutorPostInsert.java
@@ -204,9 +204,10 @@ private void executeWithId(
 		);
 
 		session.getJdbcServices().getSqlStatementLogger().logStatement( statementDetails.getSqlString() );
-		valueBindings.beforeStatement( statementDetails );
 
 		try {
+			valueBindings.beforeStatement( statementDetails );
+
 			final int affectedRowCount = session.getJdbcCoordinator()
 					.getResultSetReturn()
 					.executeUpdate( statementDetails.getStatement(), statementDetails.getSqlString() );
@@ -224,6 +225,8 @@ private void executeWithId(
 
 	@Override
 	public void release() {
+		// The mutation delegate already releases the identity insert statement
+		assert identityInsertStatementDetails.getStatement() == null;
 		secondaryTablesStatementGroup.release();
 	}
 
diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/MutationExecutorPostInsertSingleTable.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/MutationExecutorPostInsertSingleTable.java
@@ -112,7 +112,8 @@ public Object execute(
 
 	@Override
 	public void release() {
-		identityInsertStatementDetails.releaseStatement( session );
+		// Nothing to do - the mutation delegate already releases the identity insert statement
+		assert identityInsertStatementDetails.getStatement() == null;
 	}
 
 	@Override
diff --git a/hibernate-core/src/main/java/org/hibernate/id/insert/AbstractReturningDelegate.java b/hibernate-core/src/main/java/org/hibernate/id/insert/AbstractReturningDelegate.java
@@ -38,8 +38,21 @@ public Object performInsert(
 			Object entity,
 			SharedSessionContractImplementor session) {
 		session.getJdbcServices().getSqlStatementLogger().logStatement( insertStatementDetails.getSqlString() );
-		valueBindings.beforeStatement( insertStatementDetails );
-		return executeAndExtract( insertStatementDetails.getSqlString(), insertStatementDetails.getStatement(), session );
+		try {
+			valueBindings.beforeStatement( insertStatementDetails );
+			return executeAndExtract(
+					insertStatementDetails.getSqlString(),
+					insertStatementDetails.getStatement(),
+					session
+			);
+		}
+		finally {
+			if ( insertStatementDetails.getStatement() != null ) {
+				insertStatementDetails.releaseStatement( session );
+			}
+			valueBindings.afterStatement( insertStatementDetails.getMutatingTableDetails() );
+			session.getJdbcCoordinator().afterStatementExecution();
+		}
 	}
 
 	@Override
diff --git a/hibernate-core/src/main/java/org/hibernate/id/insert/AbstractSelectingDelegate.java b/hibernate-core/src/main/java/org/hibernate/id/insert/AbstractSelectingDelegate.java
@@ -70,10 +70,19 @@ public Object performInsert(
 		final JdbcServices jdbcServices = session.getJdbcServices();
 
 		jdbcServices.getSqlStatementLogger().logStatement( insertStatementDetails.getSqlString() );
-		jdbcValueBindings.beforeStatement( insertStatementDetails );
 
-		jdbcCoordinator.getResultSetReturn()
-				.executeUpdate( insertStatementDetails.resolveStatement(), insertStatementDetails.getSqlString() );
+		try {
+			jdbcValueBindings.beforeStatement( insertStatementDetails );
+			jdbcCoordinator.getResultSetReturn()
+					.executeUpdate( insertStatementDetails.resolveStatement(), insertStatementDetails.getSqlString() );
+		}
+		finally {
+			if ( insertStatementDetails.getStatement() != null ) {
+				insertStatementDetails.releaseStatement( session );
+			}
+			jdbcValueBindings.afterStatement( insertStatementDetails.getMutatingTableDetails() );
+			session.getJdbcCoordinator().afterStatementExecution();
+		}
 
 		// the insert is complete, select the generated id...
 
diff --git a/hibernate-core/src/main/java/org/hibernate/id/insert/GetGeneratedKeysDelegate.java b/hibernate-core/src/main/java/org/hibernate/id/insert/GetGeneratedKeysDelegate.java
@@ -100,10 +100,10 @@ public Object performInsert(
 
 		jdbcServices.getSqlStatementLogger().logStatement( insertSql );
 
-		final PreparedStatement insertStatement = insertStatementDetails.resolveStatement();
-		jdbcValueBindings.beforeStatement( insertStatementDetails );
-
 		try {
+			final PreparedStatement insertStatement = insertStatementDetails.resolveStatement();
+			jdbcValueBindings.beforeStatement( insertStatementDetails );
+
 			jdbcCoordinator.getResultSetReturn().executeUpdate( insertStatement, insertSql );
 
 			try {
@@ -131,16 +131,20 @@ public Object performInsert(
 					}
 				}
 			}
-			finally {
-				jdbcCoordinator.getLogicalConnection().getResourceRegistry().release( insertStatement );
+			catch (SQLException e) {
+				throw jdbcServices.getSqlExceptionHelper().convert(
+						e,
+						"Unable to extract generated-keys ResultSet",
+						insertSql
+				);
 			}
 		}
-		catch (SQLException e) {
-			throw jdbcServices.getSqlExceptionHelper().convert(
-					e,
-					"Unable to extract generated-keys ResultSet",
-					insertSql
-			);
+		finally {
+			if ( insertStatementDetails.getStatement() != null ) {
+				insertStatementDetails.releaseStatement( session );
+			}
+			jdbcValueBindings.afterStatement( insertStatementDetails.getMutatingTableDetails() );
+			jdbcCoordinator.afterStatementExecution();
 		}
 	}
 
diff --git a/hibernate-core/src/main/java/org/hibernate/id/insert/InsertReturningDelegate.java b/hibernate-core/src/main/java/org/hibernate/id/insert/InsertReturningDelegate.java
@@ -77,9 +77,6 @@ protected Object executeAndExtract(
 					insertSql
 			);
 		}
-		finally {
-			jdbcCoordinator.getLogicalConnection().getResourceRegistry().release( resultSet, insertStatement );
-		}
 	}
 
 	@Override
diff --git a/hibernate-core/src/main/java/org/hibernate/id/insert/SybaseJConnGetGeneratedKeysDelegate.java b/hibernate-core/src/main/java/org/hibernate/id/insert/SybaseJConnGetGeneratedKeysDelegate.java
@@ -73,9 +73,5 @@ public Object executeAndExtract(
 					insertSql
 			);
 		}
-		finally {
-			jdbcCoordinator.getLogicalConnection().getResourceRegistry().release( resultSet, insertStatement );
-			jdbcCoordinator.afterStatementExecution();
-		}
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -112,7 +112,8 @@ public Object execute(`
`112`	`112`
`113`	`113`	`@Override`
`114`	`114`	`public void release() {`
`115`		`- identityInsertStatementDetails.releaseStatement( session );`
	`115`	`+ // Nothing to do - the mutation delegate already releases the identity insert statement`
	`116`	`+ assert identityInsertStatementDetails.getStatement() == null;`
`116`	`117`	`}`
`117`	`118`
`118`	`119`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -100,10 +100,10 @@ public Object performInsert(`
`100`	`100`
`101`	`101`	`jdbcServices.getSqlStatementLogger().logStatement( insertSql );`
`102`	`102`
`103`		`- final PreparedStatement insertStatement = insertStatementDetails.resolveStatement();`
`104`		`- jdbcValueBindings.beforeStatement( insertStatementDetails );`
`105`		`-`
`106`	`103`	`try {`
	`104`	`+ final PreparedStatement insertStatement = insertStatementDetails.resolveStatement();`
	`105`	`+ jdbcValueBindings.beforeStatement( insertStatementDetails );`
	`106`	`+`
`107`	`107`	`jdbcCoordinator.getResultSetReturn().executeUpdate( insertStatement, insertSql );`
`108`	`108`
`109`	`109`	`try {`
`@@ -131,16 +131,20 @@ public Object performInsert(`
`131`	`131`	`}`
`132`	`132`	`}`
`133`	`133`	`}`
`134`		`- finally {`
`135`		`- jdbcCoordinator.getLogicalConnection().getResourceRegistry().release( insertStatement );`
	`134`	`+ catch (SQLException e) {`
	`135`	`+ throw jdbcServices.getSqlExceptionHelper().convert(`
	`136`	`+ e,`
	`137`	`+ "Unable to extract generated-keys ResultSet",`
	`138`	`+ insertSql`
	`139`	`+ );`
`136`	`140`	`}`
`137`	`141`	`}`
`138`		`- catch (SQLException e) {`
`139`		`- throw jdbcServices.getSqlExceptionHelper().convert(`
`140`		`- e,`
`141`		`- "Unable to extract generated-keys ResultSet",`
`142`		`- insertSql`
`143`		`- );`
	`142`	`+ finally {`
	`143`	`+ if ( insertStatementDetails.getStatement() != null ) {`
	`144`	`+ insertStatementDetails.releaseStatement( session );`
	`145`	`+ }`
	`146`	`+ jdbcValueBindings.afterStatement( insertStatementDetails.getMutatingTableDetails() );`
	`147`	`+ jdbcCoordinator.afterStatementExecution();`
`144`	`148`	`}`
`145`	`149`	`}`
`146`	`150`
Original file line number	Diff line number	Diff line change
`@@ -77,9 +77,6 @@ protected Object executeAndExtract(`
`77`	`77`	`insertSql`
`78`	`78`	`);`
`79`	`79`	`}`
`80`		`- finally {`
`81`		`- jdbcCoordinator.getLogicalConnection().getResourceRegistry().release( resultSet, insertStatement );`
`82`		`- }`
`83`	`80`	`}`
`84`	`81`
`85`	`82`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -73,9 +73,5 @@ public Object executeAndExtract(`
`73`	`73`	`insertSql`
`74`	`74`	`);`
`75`	`75`	`}`
`76`		`- finally {`
`77`		`- jdbcCoordinator.getLogicalConnection().getResourceRegistry().release( resultSet, insertStatement );`
`78`		`- jdbcCoordinator.afterStatementExecution();`
`79`		`- }`
`80`	`76`	`}`
`81`	`77`	`}`