Skip to content

Commit 0e164d1

Browse files
sebersolesebersole
committed
HHH-7689 - addToBatch in BatchingBatch blows out of error handling and doesn't abort the batch
1 parent ca3b22a commit 0e164d1

File tree

5 files changed

+188
-20
lines changed

5 files changed

+188
-20
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ protected SqlStatementLogger sqlStatementLogger() {
101101
.getSqlStatementLogger();
102102
}
103103

104+
protected void abortBatch() {
105+
jdbcCoordinator.abortBatch();
106+
}
107+
104108
/**
105109
* Access to the batch's map of statements (keyed by SQL statement string).
106110
*

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,8 @@ private void performExecution() {
119119
checkRowCounts( statement.executeBatch(), statement );
120120
}
121121
catch ( SQLException e ) {
122-
LOG.debug( "SQLException escaped proxy", e );
123-
throw sqlExceptionHelper().convert( e, "could not perform addBatch", entry.getKey() );
122+
abortBatch();
123+
throw sqlExceptionHelper().convert( e, "could not execute batch", entry.getKey() );
124124
}
125125
}
126126
}

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.sql.SQLException;
2828
import java.util.Map;
2929

30+
import org.hibernate.JDBCException;
3031
import org.hibernate.engine.jdbc.batch.spi.BatchKey;
3132
import org.hibernate.engine.jdbc.spi.JdbcCoordinator;
3233
import org.hibernate.internal.CoreMessageLogger;
@@ -62,8 +63,12 @@ public void addToBatch() {
6263
jdbcCoordinator.release( statement );
6364
}
6465
catch ( SQLException e ) {
65-
LOG.debug( "SQLException escaped proxy", e );
66-
throw sqlExceptionHelper().convert( e, "could not execute batch statement", entry.getKey() );
66+
abortBatch();
67+
throw sqlExceptionHelper().convert( e, "could not execute non-batched batch statement", entry.getKey() );
68+
}
69+
catch (JDBCException e) {
70+
abortBatch();
71+
throw e;
6772
}
6873
}
6974
getStatements().clear();

‎hibernate-core/src/test/java/org/hibernate/test/batch/BatchingBatchFailureTest.java

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import java.lang.reflect.Field;
3232
import java.util.Map;
3333

34-
import org.hibernate.HibernateException;
3534
import org.hibernate.Session;
3635
import org.hibernate.cfg.AvailableSettings;
3736
import org.hibernate.cfg.Configuration;
@@ -73,22 +72,20 @@ public void testBasicInsertion() {
7372
Session session = openSession();
7473
session.getTransaction().begin();
7574

76-
//open secondary session to insert a valid entity and an invalid entity
77-
Session secondSession = session.sessionWithOptions()
78-
.connection()
79-
.flushBeforeCompletion( true )
80-
.autoClose( true )
81-
.openSession();
82-
8375
try {
84-
secondSession.persist( new User( 1, "ok" ) );
85-
secondSession.persist( new User( 2, null ) );
76+
session.persist( new User( 1, "ok" ) );
77+
session.persist( new User( 2, null ) );
78+
session.persist( new User( 3, "ok" ) );
79+
session.persist( new User( 4, "ok" ) );
80+
session.persist( new User( 5, "ok" ) );
81+
session.persist( new User( 6, "ok" ) );
8682
// the flush should fail
87-
secondSession.flush();
88-
fail( "Expecting SQLException" );
83+
session.flush();
84+
fail( "Expecting failed flush" );
8985
}
90-
catch (HibernateException expected) {
91-
secondSession.close();
86+
catch (Exception expected) {
87+
System.out.println( "Caught expected exception : " + expected );
88+
expected.printStackTrace( System.out );
9289

9390
try {
9491
//at this point the transaction is still active but the batch should have been aborted (have to use reflection to get at the field)
@@ -121,9 +118,7 @@ public void testBasicInsertion() {
121118
@Entity( name = "User" )
122119
@Table( name = "`USER`" )
123120
public static class User {
124-
@Id
125121
private Integer id;
126-
@Column( nullable = false )
127122
private String name;
128123

129124
public User() {
@@ -133,5 +128,23 @@ public User(Integer id, String name) {
133128
this.id = id;
134129
this.name = name;
135130
}
131+
132+
@Id
133+
public Integer getId() {
134+
return id;
135+
}
136+
137+
public void setId(Integer id) {
138+
this.id = id;
139+
}
140+
141+
@Column( nullable = false )
142+
public String getName() {
143+
return name;
144+
}
145+
146+
public void setName(String name) {
147+
this.name = name;
148+
}
136149
}
137150
}

‎hibernate-core/src/test/java/org/hibernate/test/batch/NonBatchingBatchFailureTest.java

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/*
2+
* Hibernate, Relational Persistence for Idiomatic Java
3+
*
4+
* Copyright (c) 2013, Red Hat Inc. or third-party contributors as
5+
* indicated by the @author tags or express copyright attribution
6+
* statements applied by the authors. All third-party contributions are
7+
* distributed under license by Red Hat Inc.
8+
*
9+
* This copyrighted material is made available to anyone wishing to use, modify,
10+
* copy, or redistribute it subject to the terms and conditions of the GNU
11+
* Lesser General Public License, as published by the Free Software Foundation.
12+
*
13+
* This program is distributed in the hope that it will be useful,
14+
* but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
15+
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
16+
* for more details.
17+
*
18+
* You should have received a copy of the GNU Lesser General Public License
19+
* along with this distribution; if not, write to:
20+
* Free Software Foundation, Inc.
21+
* 51 Franklin Street, Fifth Floor
22+
* Boston, MA 02110-1301 USA
23+
*/
24+
package org.hibernate.test.batch;
25+
26+
import javax.persistence.Column;
27+
import javax.persistence.Entity;
28+
import javax.persistence.Id;
29+
import javax.persistence.Table;
30+
import java.lang.reflect.Field;
31+
import java.util.Map;
32+
33+
import org.hibernate.Session;
34+
import org.hibernate.cfg.AvailableSettings;
35+
import org.hibernate.cfg.Configuration;
36+
import org.hibernate.engine.jdbc.batch.internal.AbstractBatchImpl;
37+
import org.hibernate.engine.jdbc.batch.internal.NonBatchingBatch;
38+
import org.hibernate.engine.jdbc.batch.spi.Batch;
39+
import org.hibernate.engine.spi.SessionImplementor;
40+
41+
import org.junit.Test;
42+
43+
import org.hibernate.testing.TestForIssue;
44+
import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
45+
46+
import static org.junit.Assert.assertEquals;
47+
import static org.junit.Assert.fail;
48+
49+
/**
50+
* @author Shawn Clowater
51+
* @author Steve Ebersole
52+
*/
53+
@TestForIssue( jiraKey = "HHH-7689" )
54+
public class NonBatchingBatchFailureTest extends BaseCoreFunctionalTestCase {
55+
@Override
56+
protected Class<?>[] getAnnotatedClasses() {
57+
return new Class[] { User.class };
58+
}
59+
60+
@Override
61+
protected void configure(Configuration configuration) {
62+
super.configure( configuration );
63+
// explicitly disable batching
64+
configuration.setProperty( AvailableSettings.STATEMENT_BATCH_SIZE, "-1" );
65+
// and disable in-vm nullability checking (so we can force in-db not-null constraint violations)
66+
configuration.setProperty( AvailableSettings.CHECK_NULLABILITY, "false" );
67+
}
68+
69+
@Test
70+
public void testBasicInsertion() {
71+
Session session = openSession();
72+
session.getTransaction().begin();
73+
74+
try {
75+
session.persist( new User( 1, "ok" ) );
76+
session.persist( new User( 2, null ) );
77+
session.persist( new User( 3, "ok" ) );
78+
// the flush should fail
79+
session.flush();
80+
fail( "Expecting failed flush" );
81+
}
82+
catch (Exception expected) {
83+
System.out.println( "Caught expected exception : " + expected );
84+
expected.printStackTrace( System.out );
85+
86+
try {
87+
//at this point the transaction is still active but the batch should have been aborted (have to use reflection to get at the field)
88+
SessionImplementor sessionImplementor = (SessionImplementor) session;
89+
Field field = sessionImplementor.getTransactionCoordinator().getJdbcCoordinator().getClass().getDeclaredField( "currentBatch" );
90+
field.setAccessible( true );
91+
Batch batch = (Batch) field.get( sessionImplementor.getTransactionCoordinator().getJdbcCoordinator() );
92+
if ( batch == null ) {
93+
throw new Exception( "Current batch was null" );
94+
}
95+
else {
96+
//make sure it's actually a batching impl
97+
assertEquals( NonBatchingBatch.class, batch.getClass() );
98+
field = AbstractBatchImpl.class.getDeclaredField( "statements" );
99+
field.setAccessible( true );
100+
//check to see that there aren't any statements queued up (this can be an issue if using SavePoints)
101+
assertEquals( 0, ((Map) field.get( batch )).size() );
102+
}
103+
}
104+
catch (Exception fieldException) {
105+
fail( "Couldn't inspect field " + fieldException.getMessage() );
106+
}
107+
}
108+
finally {
109+
session.getTransaction().rollback();
110+
session.close();
111+
}
112+
}
113+
114+
@Entity( name = "User" )
115+
@Table( name = "`USER`" )
116+
public static class User {
117+
private Integer id;
118+
private String name;
119+
120+
public User() {
121+
}
122+
123+
public User(Integer id, String name) {
124+
this.id = id;
125+
this.name = name;
126+
}
127+
128+
@Id
129+
public Integer getId() {
130+
return id;
131+
}
132+
133+
public void setId(Integer id) {
134+
this.id = id;
135+
}
136+
137+
@Column( nullable = false )
138+
public String getName() {
139+
return name;
140+
}
141+
142+
public void setName(String name) {
143+
this.name = name;
144+
}
145+
}
146+
}

0 commit comments

Comments
 (0)