Skip to content

Commit

Permalink
Fix for Bug#104067 (33054827), No reset autoCommit after unknown issu…
Browse files Browse the repository at this point in the history
…e occurs.
  • Loading branch information
fjssilva committed Nov 19, 2021
1 parent bc45d35 commit 4d19ea1
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

Version 8.0.28

- Fix for Bug#104067 (33054827), No reset autoCommit after unknown issue occurs.
Thanks to Tingyu Wei for his contribution.

- Fix for Bug#85223 (25656020), MYSQLSQLXML SETSTRING CRASH.

- Fix for Bug#84365 (33425867), INSERT..VALUE with VALUES function lead to a StringIndexOutOfBoundsException.
Expand Down
12 changes: 10 additions & 2 deletions src/main/user-impl/java/com/mysql/cj/jdbc/ConnectionImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
import com.mysql.cj.conf.PropertyDefinitions.DatabaseTerm;
import com.mysql.cj.conf.PropertyKey;
import com.mysql.cj.conf.RuntimeProperty;
import com.mysql.cj.exceptions.CJCommunicationsException;
import com.mysql.cj.exceptions.CJException;
import com.mysql.cj.exceptions.ExceptionFactory;
import com.mysql.cj.exceptions.ExceptionInterceptor;
Expand Down Expand Up @@ -2017,10 +2018,10 @@ void forEach(ConnectionLifecycleInterceptor each) throws SQLException {
this.autoReconnect.setValue(true);
}

boolean isAutocommit = this.session.getServerSession().isAutocommit();

This comment has been minimized.

Copy link
@sgrachov

sgrachov Feb 10, 2022

possible mistype. should have been isAutoCommit() not isAutocommit(). would be nice if methods were named differently.

This comment has been minimized.

Copy link
@fjssilva

fjssilva Feb 10, 2022

Author Contributor

Actually it works both ways, but you are right, isAutoCommit() matches more accurately the underlying logic. This will be fixed. Thanks!

This comment has been minimized.

Copy link
@ztyzbb

ztyzbb May 13, 2022

When connection used in pools with useLocalSessionState=true, this will cause all query in a transaction auto commited, so transaction can't rollback. And connection pool like HikariCP recommend set useLocalSessionState to true😢

This comment has been minimized.

Copy link
@ThomasNimstad

ThomasNimstad May 13, 2022

Looks to be fixed in 8.0.29

try {
boolean needsSetOnServer = true;

if (this.useLocalSessionState.getValue() && this.session.getServerSession().isAutoCommit() == autoCommitFlag) {
if (this.useLocalSessionState.getValue() && isAutocommit == autoCommitFlag) {
needsSetOnServer = false;
} else if (!this.autoReconnect.getValue()) {
needsSetOnServer = getSession().isSetNeededForAutoCommitMode(autoCommitFlag);
Expand All @@ -2035,6 +2036,13 @@ void forEach(ConnectionLifecycleInterceptor each) throws SQLException {
this.session.execSQL(null, autoCommitFlag ? "SET autocommit=1" : "SET autocommit=0", -1, null, false, this.nullStatementResultSetFactory,
null, false);
}
} catch (CJCommunicationsException e) {
throw e;
} catch (CJException e) {
// Reset to current autocommit value in case of an exception different than a communication exception occurs.
this.session.getServerSession().setAutoCommit(isAutocommit);
// Update the stacktrace.
throw SQLError.createSQLException(e.getMessage(), e.getSQLState(), e.getVendorCode(), e.isTransient(), e, getExceptionInterceptor());
} finally {
if (this.autoReconnectForPools.getValue()) {
this.autoReconnect.setValue(false);
Expand Down
65 changes: 65 additions & 0 deletions src/test/java/testsuite/regression/ConnectionRegressionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11562,4 +11562,69 @@ public void testBug28725534() throws Exception {

assertTrue(end - start < 250, ".equals() took too long to exectute, the method is being locked by a synchronized block.");
}

/**
* Tests fix for Bug#104067 (33054827), No reset autoCommit after unknown issue occurs.
*
* @throws Exception
*/
@Test
public void testBug104067() throws Exception {
Properties props = new Properties();
props.setProperty(PropertyKey.sslMode.getKeyName(), SslMode.DISABLED.name());
props.setProperty(PropertyKey.allowPublicKeyRetrieval.getKeyName(), "true");
props.setProperty(PropertyKey.queryInterceptors.getKeyName(), Bug104067QueryInterceptor.class.getName());
Connection testConn = getConnectionWithProps(props);
Statement testStmt = testConn.createStatement();

// Connection vs session autocommit value:
// 1. Default value.
assertTrue(testConn.getAutoCommit());
this.rs = testStmt.executeQuery("SHOW SESSION VARIABLES LIKE 'autocommit'");
assertTrue(this.rs.next());
assertTrue(this.rs.getString(2).equalsIgnoreCase("ON"));

// 2. After Connection.setAutcommit(true).
try {
testConn.setAutoCommit(true);
} catch (SQLException e) {
fail("Exception not expected.", e);
}
assertTrue(testConn.getAutoCommit());
this.rs = testStmt.executeQuery("SHOW SESSION VARIABLES LIKE 'autocommit'");
assertTrue(this.rs.next());
assertTrue(this.rs.getString(2).equalsIgnoreCase("ON"));

// 3. After Connection.setAutcommit(false).
assertThrows(SQLException.class, () -> {
testConn.setAutoCommit(false);
return null;
});
assertTrue(testConn.getAutoCommit());
this.rs = testStmt.executeQuery("SHOW SESSION VARIABLES LIKE 'autocommit'");
this.rs.next();
assertTrue(this.rs.getString(2).equalsIgnoreCase("ON"));

// 4. After Connection.setAutcommit(true).
try {
testConn.setAutoCommit(true);
} catch (SQLException e) {
fail("Exception not expected.", e);
}
assertTrue(testConn.getAutoCommit());
this.rs = testStmt.executeQuery("SHOW SESSION VARIABLES LIKE 'autocommit'");
assertTrue(this.rs.next());
assertTrue(this.rs.getString(2).equalsIgnoreCase("ON"));
}

public static class Bug104067QueryInterceptor extends BaseQueryInterceptor {
@Override
public <T extends Resultset> T preProcess(Supplier<String> str, Query interceptedQuery) {
String sql = str.get();
if (sql.equalsIgnoreCase("SET autocommit=0")) {
throw ExceptionFactory.createException("Artificial non-connection related exception while executing \"" + sql + "\"");
}
return super.preProcess(str, interceptedQuery);
}
}
}

0 comments on commit 4d19ea1

Please sign in to comment.