From 8a314530bff4a10beb9b14db2629174233373a64 Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 9 May 2017 12:33:37 +0200 Subject: [PATCH] [misc] changing default option value useServerPrepStmts to false (use text protocol by default) --- .travis.yml | 3 +- .travis/script.sh | 11 ++- .../jdbc/MariaDbPreparedStatementClient.java | 7 +- .../jdbc/internal/util/DefaultOptions.java | 4 +- .../org/mariadb/jdbc/ErrorMessageTest.java | 10 ++- .../mariadb/jdbc/PreparedStatementTest.java | 2 +- .../java/org/mariadb/jdbc/RePrepareTest.java | 88 +++++++++++++++++++ 7 files changed, 109 insertions(+), 16 deletions(-) create mode 100644 src/test/java/org/mariadb/jdbc/RePrepareTest.java diff --git a/.travis.yml b/.travis.yml index 07564bc1a..5baca1557 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,10 +22,11 @@ env: - MARIA=10.2 PACKET=8M - MARIA=10.2 PACKET=20M - MARIA=10.2 PACKET=40M + - TYPE=PREPARE MARIA=10.1 PACKET=40M - TYPE=MAXSCALE MAXSCALE_VERSION=2.0.2 MARIA=10.1 PACKET=40M - TYPE=REWRITE MARIA=10.1 PACKET=40M - TYPE=MULTI MARIA=10.1 PACKET=40M - - TYPE=BULK_CLIENT MARIA=10.1 PACKET=40M + - TYPE=BULK_SERVER MARIA=10.1 PACKET=40M - TYPE=NO_BULK_CLIENT MARIA=10.1 PACKET=40M - TYPE=NO_BULK_SERVER MARIA=10.1 PACKET=40M - COMPRESSION=false MARIA=10.1 PACKET=40M diff --git a/.travis/script.sh b/.travis/script.sh index 0441952d2..8e94014bf 100644 --- a/.travis/script.sh +++ b/.travis/script.sh @@ -10,17 +10,20 @@ case "$TYPE" in "REWRITE" ) urlString=-DdbUrl='jdbc:mariadb://localhost:3306/testj?user=root&rewriteBatchedStatements=true' ;; + "PREPARE" ) + urlString=-DdbUrl='jdbc:mariadb://localhost:3306/testj?user=root&useServerPrepStmts=true' + ;; "MULTI" ) urlString=-DdbUrl='jdbc:mariadb://localhost:3306/testj?user=root&allowMultiQueries=true' ;; - "BULK_CLIENT" ) - urlString=-DdbUrl='jdbc:mariadb://localhost:3306/testj?user=root&useBatchMultiSend=true&useServerPrepStmts=false' + "BULK_SERVER" ) + urlString=-DdbUrl='jdbc:mariadb://localhost:3306/testj?user=root&useBatchMultiSend=true&useServerPrepStmts=true' ;; "NO_BULK_CLIENT" ) - urlString=-DdbUrl='jdbc:mariadb://localhost:3306/testj?user=root&useBatchMultiSend=false&useServerPrepStmts=false' + urlString=-DdbUrl='jdbc:mariadb://localhost:3306/testj?user=root&useBatchMultiSend=false' ;; "NO_BULK_SERVER" ) - urlString=-DdbUrl='jdbc:mariadb://localhost:3306/testj?user=root&useBatchMultiSend=false' + urlString=-DdbUrl='jdbc:mariadb://localhost:3306/testj?user=root&useBatchMultiSend=false&useServerPrepStmts=true' ;; "COMPRESSION" ) urlString=-DdbUrl='jdbc:mariadb://localhost:3306/testj?user=root&useCompression=true' diff --git a/src/main/java/org/mariadb/jdbc/MariaDbPreparedStatementClient.java b/src/main/java/org/mariadb/jdbc/MariaDbPreparedStatementClient.java index 9de7750ea..f0fa58e7d 100644 --- a/src/main/java/org/mariadb/jdbc/MariaDbPreparedStatementClient.java +++ b/src/main/java/org/mariadb/jdbc/MariaDbPreparedStatementClient.java @@ -188,10 +188,9 @@ protected boolean executeInternal(int fetchSize) throws SQLException { //valid parameters for (int i = 0; i < prepareResult.getParamCount(); i++) { if (parameters[i] == null) { - logger.error("You need to set exactly " + prepareResult.getParamCount() - + " parameters on the prepared statement"); - throw ExceptionMapper.getSqlException("You need to set exactly " + prepareResult.getParamCount() - + " parameters on the prepared statement"); + logger.error("Parameter at position " + (i + 1) + " is not set"); + ExceptionMapper.throwException(new SQLException("Parameter at position " + (i + 1) + " is not set", "07004"), + connection, this); } } diff --git a/src/main/java/org/mariadb/jdbc/internal/util/DefaultOptions.java b/src/main/java/org/mariadb/jdbc/internal/util/DefaultOptions.java index 47536e7ad..67d13760e 100644 --- a/src/main/java/org/mariadb/jdbc/internal/util/DefaultOptions.java +++ b/src/main/java/org/mariadb/jdbc/internal/util/DefaultOptions.java @@ -296,9 +296,9 @@ public enum DefaultOptions { /** * useServerPrepStmts must prepared statements be prepared on server side, or just faked on client side. * if rewriteBatchedStatements is set to true, this options will be set to false. - * default to true. + * default to false. */ - USESERVERPREPSTMTS("useServerPrepStmts", Boolean.TRUE, "1.3.0"), + USESERVERPREPSTMTS("useServerPrepStmts", Boolean.FALSE, "1.3.0"), /** * File path of the trustStore file (similar to java System property "javax.net.ssl.trustStore"). diff --git a/src/test/java/org/mariadb/jdbc/ErrorMessageTest.java b/src/test/java/org/mariadb/jdbc/ErrorMessageTest.java index 97563d7ea..6b84383cc 100644 --- a/src/test/java/org/mariadb/jdbc/ErrorMessageTest.java +++ b/src/test/java/org/mariadb/jdbc/ErrorMessageTest.java @@ -62,8 +62,9 @@ public void testSmallBulkErrorMessage() throws SQLException { executeBatchWithException(connection); fail("Must Have thrown error"); } catch (SQLException sqle) { - assertTrue(sqle.getCause().getCause().getMessage().contains( - "INSERT INTO testErrorMessage(test, test2) values (?, ?)")); + String query = "INSERT INTO testErrorMessage(test, test2) values (" + + (sharedUsePrepare() ? "?, ?)" : "'more than 10 characters to provoc error', 10)"); + assertTrue(sqle.getCause().getCause().getMessage().contains(query)); } } @@ -109,8 +110,9 @@ public void testBigBulkErrorMessage() throws SQLException { executeBigBatchWithException(connection); fail("Must Have thrown error"); } catch (SQLException sqle) { - assertTrue(sqle.getCause().getCause().getMessage().contains( - "INSERT INTO testErrorMessage(test, test2) values (?, ?)")); + String query = "INSERT INTO testErrorMessage(test, test2) values (" + + (sharedUsePrepare() ? "?, ?)" : "'more than 10 characters to provoc error', 200)"); + assertTrue(sqle.getCause().getCause().getMessage().contains(query)); } } diff --git a/src/test/java/org/mariadb/jdbc/PreparedStatementTest.java b/src/test/java/org/mariadb/jdbc/PreparedStatementTest.java index 79e031e29..f1c501b56 100644 --- a/src/test/java/org/mariadb/jdbc/PreparedStatementTest.java +++ b/src/test/java/org/mariadb/jdbc/PreparedStatementTest.java @@ -297,7 +297,7 @@ private void testRewriteMultiPacket(boolean notRewritable) throws SQLException { } int[] results = pstmt.executeBatch(); assertEquals(2, results.length); - if (notRewritable && !sharedIsRewrite()) { + if (notRewritable) { for (int result : results) assertEquals(1, result); } else { for (int result : results) assertEquals(Statement.SUCCESS_NO_INFO, result); diff --git a/src/test/java/org/mariadb/jdbc/RePrepareTest.java b/src/test/java/org/mariadb/jdbc/RePrepareTest.java new file mode 100644 index 000000000..4c35930a7 --- /dev/null +++ b/src/test/java/org/mariadb/jdbc/RePrepareTest.java @@ -0,0 +1,88 @@ +package org.mariadb.jdbc; + +import org.junit.Test; + +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; + +import static org.junit.Assert.*; + +public class RePrepareTest extends BaseTest { + + @Test + public void rePrepareTestSelectError() throws SQLException { + createTable("rePrepareTestSelectError", "test int"); + try (Statement stmt = sharedConnection.createStatement()) { + stmt.execute("INSERT INTO rePrepareTestSelectError(test) VALUES (1)"); + try (PreparedStatement preparedStatement = sharedConnection.prepareStatement("SELECT * FROM rePrepareTestSelectError where test = ?")) { + preparedStatement.setInt(1, 1); + ResultSet rs = preparedStatement.executeQuery(); + assertTrue(rs.next()); + assertEquals(1, rs.getInt(1)); + assertFalse(rs.next()); + stmt.execute("ALTER TABLE rePrepareTestSelectError" + + " CHANGE COLUMN `test` `test` VARCHAR(50) NULL DEFAULT NULL FIRST," + + "ADD COLUMN `test2` VARCHAR(50) NULL DEFAULT NULL AFTER `test`;"); + ResultSet rs2 = preparedStatement.executeQuery(); + preparedStatement.setInt(1, 1); + assertTrue(rs2.next()); + assertEquals("1", rs2.getString(1)); + assertFalse(rs2.next()); + } + } + } + + @Test + public void rePrepareTestInsertError() throws SQLException { + createTable("rePrepareTestInsertError", "test int"); + try (Statement stmt = sharedConnection.createStatement()) { + try (PreparedStatement preparedStatement = sharedConnection.prepareStatement("INSERT INTO rePrepareTestInsertError(test) values (?)")) { + + preparedStatement.setInt(1, 1); + preparedStatement.execute(); + + stmt.execute("ALTER TABLE rePrepareTestInsertError" + + " CHANGE COLUMN `test` `test` VARCHAR(50) NULL DEFAULT NULL FIRST;"); + + preparedStatement.setInt(1, 2); + preparedStatement.execute(); + + stmt.execute("ALTER TABLE rePrepareTestInsertError" + + " CHANGE COLUMN `test` `test` VARCHAR(100) NULL DEFAULT NULL FIRST," + + "ADD COLUMN `test2` VARCHAR(50) NULL DEFAULT NULL AFTER `test`;"); + + stmt.execute("flush tables with read lock"); + stmt.execute("unlock tables"); + preparedStatement.setInt(1, 3); + preparedStatement.execute(); + } + } + } + + + @Test + public void cannotRePrepare() throws SQLException { + createTable("cannotRePrepare", "test int"); + try (Statement stmt = sharedConnection.createStatement()) { + try (PreparedStatement preparedStatement = sharedConnection.prepareStatement("INSERT INTO cannotRePrepare(test) values (?)")) { + + preparedStatement.setInt(1, 1); + preparedStatement.execute(); + + stmt.execute("ALTER TABLE cannotRePrepare" + + " CHANGE COLUMN `test` `otherName` VARCHAR(50) NULL DEFAULT NULL FIRST;"); + + preparedStatement.setInt(1, 2); + try { + preparedStatement.execute(); + fail(); + } catch (SQLException sqle) { + assertTrue(sqle.getMessage().contains("Unknown column 'test' in 'field list'")); + } + + } + } + } +}