From 97f39caef64c90bd1b1d0aedabc0a6a3bc858edf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Fri, 14 Feb 2025 12:11:21 +0100 Subject: [PATCH 1/2] perf: use last_statement optimization in autocommit Statements that are executed in autocommit mode should use the last_statement optimization. --- .../spanner/jdbc/ExecuteMockServerTest.java | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/src/test/java/com/google/cloud/spanner/jdbc/ExecuteMockServerTest.java b/src/test/java/com/google/cloud/spanner/jdbc/ExecuteMockServerTest.java index d7d72f76f..926acc7fa 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/ExecuteMockServerTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/ExecuteMockServerTest.java @@ -38,6 +38,7 @@ import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; import com.google.spanner.v1.CommitRequest; import com.google.spanner.v1.ExecuteBatchDmlRequest; +import com.google.spanner.v1.ExecuteSqlRequest; import com.google.spanner.v1.ResultSetMetadata; import com.google.spanner.v1.ResultSetStats; import com.google.spanner.v1.StructType; @@ -870,4 +871,104 @@ public void testExecuteAutoBatchDml() throws SQLException { assertEquals(3, request.getStatementsCount()); assertEquals(1, mockSpanner.countRequestsOfType(CommitRequest.class)); } + + @Test + public void testLastStatement_AutoCommit_Query() throws SQLException { + try (Connection connection = createJdbcConnection(); + Statement statement = connection.createStatement()) { + //noinspection EmptyTryBlock + try (ResultSet ignore = statement.executeQuery(query)) { + } + } + assertEquals(1, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); + assertFalse(mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0).getLastStatement()); + } + + @Test + public void testLastStatement_AutoCommit_Dml() throws SQLException { + try (Connection connection = createJdbcConnection(); + Statement statement = connection.createStatement()) { + statement.executeUpdate(dml); + } + assertEquals(1, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); + assertTrue(mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0).getLastStatement()); + } + + @Test + public void testLastStatement_AutoCommit_DmlReturning() throws SQLException { + try (Connection connection = createJdbcConnection(); + Statement statement = connection.createStatement()) { + //noinspection EmptyTryBlock + try (ResultSet ignore = statement.executeQuery(dmlReturning)) { + } + } + assertEquals(1, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); + assertTrue(mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0).getLastStatement()); + } + + @Test + public void testLastStatement_AutoCommit_BatchDml() throws SQLException { + try (Connection connection = createJdbcConnection(); + Statement statement = connection.createStatement()) { + statement.addBatch(dml); + statement.addBatch(dml); + statement.executeBatch(); + } + assertEquals(1, mockSpanner.countRequestsOfType(ExecuteBatchDmlRequest.class)); + assertTrue(mockSpanner.getRequestsOfType(ExecuteBatchDmlRequest.class).get(0).getLastStatements()); + } + + @Test + public void testLastStatement_Transaction_Query() throws SQLException { + try (Connection connection = createJdbcConnection(); + Statement statement = connection.createStatement()) { + connection.setAutoCommit(false); + //noinspection EmptyTryBlock + try (ResultSet ignore = statement.executeQuery(query)) { + } + connection.commit(); + } + assertEquals(1, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); + assertFalse(mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0).getLastStatement()); + } + + @Test + public void testLastStatement_Transaction_Dml() throws SQLException { + try (Connection connection = createJdbcConnection(); + Statement statement = connection.createStatement()) { + connection.setAutoCommit(false); + statement.executeUpdate(dml); + connection.commit(); + } + assertEquals(1, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); + assertFalse(mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0).getLastStatement()); + } + + @Test + public void testLastStatement_Transaction_DmlReturning() throws SQLException { + try (Connection connection = createJdbcConnection(); + Statement statement = connection.createStatement()) { + connection.setAutoCommit(false); + //noinspection EmptyTryBlock + try (ResultSet ignore = statement.executeQuery(dmlReturning)) { + } + connection.commit(); + } + assertEquals(1, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); + assertFalse(mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0).getLastStatement()); + } + + @Test + public void testLastStatement_Transaction_BatchDml() throws SQLException { + try (Connection connection = createJdbcConnection(); + Statement statement = connection.createStatement()) { + connection.setAutoCommit(false); + statement.addBatch(dml); + statement.addBatch(dml); + statement.executeBatch(); + connection.commit(); + } + assertEquals(1, mockSpanner.countRequestsOfType(ExecuteBatchDmlRequest.class)); + assertFalse(mockSpanner.getRequestsOfType(ExecuteBatchDmlRequest.class).get(0).getLastStatements()); + } } From 7e7c54cdc0ecb14928289631c44b9fcfd55ee8de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Fri, 21 Feb 2025 12:16:00 +0100 Subject: [PATCH 2/2] test: add more tests --- .../spanner/jdbc/ExecuteMockServerTest.java | 89 ++++++++++++++++--- 1 file changed, 78 insertions(+), 11 deletions(-) diff --git a/src/test/java/com/google/cloud/spanner/jdbc/ExecuteMockServerTest.java b/src/test/java/com/google/cloud/spanner/jdbc/ExecuteMockServerTest.java index 926acc7fa..0da181444 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/ExecuteMockServerTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/ExecuteMockServerTest.java @@ -67,6 +67,8 @@ */ @RunWith(Parameterized.class) public class ExecuteMockServerTest extends AbstractMockServerTest { + private static final IllegalStateException REQUEST_NOT_FOUND = + new IllegalStateException("request not found"); private static Dialect currentDialect; @Parameters(name = "dialect = {0}") @@ -220,9 +222,27 @@ public void testStatementExecuteQuery() throws SQLException { try (ResultSet resultSet = statement.executeQuery(query)) { verifyResultSet(resultSet); } + ExecuteSqlRequest request = + mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).stream() + .filter(r -> r.getSql().equals(query)) + .findAny() + .orElseThrow(() -> REQUEST_NOT_FOUND); + assertTrue(request.getTransaction().hasSingleUse()); + assertTrue(request.getTransaction().getSingleUse().hasReadOnly()); + assertFalse(request.getLastStatement()); + try (ResultSet resultSet = statement.executeQuery(dmlReturning)) { verifyResultSet(resultSet); } + request = + mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).stream() + .filter(r -> r.getSql().equals(dmlReturning)) + .findAny() + .orElseThrow(() -> REQUEST_NOT_FOUND); + assertTrue(request.getTransaction().hasBegin()); + assertTrue(request.getTransaction().getBegin().hasReadWrite()); + assertTrue(request.getLastStatement()); + try (ResultSet resultSet = statement.executeQuery(clientSideQuery)) { verifyClientSideResultSet(resultSet); } @@ -238,6 +258,15 @@ public void testStatementExecuteUpdate() throws SQLException { try (Connection connection = createJdbcConnection(); Statement statement = connection.createStatement()) { assertEquals(1, statement.executeUpdate(dml)); + ExecuteSqlRequest request = + mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).stream() + .filter(r -> r.getSql().equals(dml)) + .findAny() + .orElseThrow(() -> REQUEST_NOT_FOUND); + assertTrue(request.getTransaction().hasBegin()); + assertTrue(request.getTransaction().getBegin().hasReadWrite()); + assertTrue(request.getLastStatement()); + assertEquals(0, statement.executeUpdate(DDL)); connection.setAutoCommit(false); @@ -257,6 +286,14 @@ public void testStatementExecuteUpdateReturnGeneratedKeys() throws SQLException Statement statement = connection.createStatement()) { // TODO: Add tests for RETURN_GENERATED_KEYS when that is supported. assertEquals(1, statement.executeUpdate(dml, Statement.NO_GENERATED_KEYS)); + ExecuteSqlRequest request = + mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).stream() + .findAny() + .orElseThrow(() -> REQUEST_NOT_FOUND); + assertTrue(request.getTransaction().hasBegin()); + assertTrue(request.getTransaction().getBegin().hasReadWrite()); + assertTrue(request.getLastStatement()); + assertEquals(0, statement.executeUpdate(DDL, Statement.NO_GENERATED_KEYS)); assertEquals(0, statement.executeUpdate(clientSideUpdate, Statement.NO_GENERATED_KEYS)); @@ -272,6 +309,14 @@ public void testStatementExecuteUpdateReturnColumnNames() throws SQLException { try (Connection connection = createJdbcConnection(); Statement statement = connection.createStatement()) { assertEquals(1, statement.executeUpdate(dml, new String[] {"id"})); + ExecuteSqlRequest request = + mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).stream() + .findAny() + .orElseThrow(() -> REQUEST_NOT_FOUND); + assertTrue(request.getTransaction().hasBegin()); + assertTrue(request.getTransaction().getBegin().hasReadWrite()); + assertTrue(request.getLastStatement()); + assertEquals(0, statement.executeUpdate(DDL, new String[] {"id"})); assertEquals(0, statement.executeUpdate(clientSideUpdate, new String[] {"id"})); @@ -290,6 +335,14 @@ public void testStatementExecuteUpdateReturnColumnIndexes() throws SQLException try (Connection connection = createJdbcConnection(); Statement statement = connection.createStatement()) { assertEquals(1, statement.executeUpdate(dml, new int[] {1})); + ExecuteSqlRequest request = + mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).stream() + .findAny() + .orElseThrow(() -> REQUEST_NOT_FOUND); + assertTrue(request.getTransaction().hasBegin()); + assertTrue(request.getTransaction().getBegin().hasReadWrite()); + assertTrue(request.getLastStatement()); + assertEquals(0, statement.executeUpdate(DDL, new int[] {1})); assertEquals(0, statement.executeUpdate(clientSideUpdate, new int[] {1})); verifyOverflow(() -> statement.executeUpdate(largeDml, new int[] {1})); @@ -304,6 +357,14 @@ public void testStatementLargeExecuteUpdate() throws SQLException { try (Connection connection = createJdbcConnection(); Statement statement = connection.createStatement()) { assertEquals(1L, statement.executeLargeUpdate(dml)); + ExecuteSqlRequest request = + mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).stream() + .findAny() + .orElseThrow(() -> REQUEST_NOT_FOUND); + assertTrue(request.getTransaction().hasBegin()); + assertTrue(request.getTransaction().getBegin().hasReadWrite()); + assertTrue(request.getLastStatement()); + assertEquals(0L, statement.executeLargeUpdate(DDL)); assertEquals(0L, statement.executeLargeUpdate(clientSideUpdate)); assertEquals(LARGE_UPDATE_COUNT, statement.executeLargeUpdate(largeDml)); @@ -319,6 +380,14 @@ public void testStatementExecuteLargeUpdateReturnGeneratedKeys() throws SQLExcep Statement statement = connection.createStatement()) { // TODO: Add tests for RETURN_GENERATED_KEYS when that is supported. assertEquals(1, statement.executeLargeUpdate(dml, Statement.NO_GENERATED_KEYS)); + ExecuteSqlRequest request = + mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).stream() + .findAny() + .orElseThrow(() -> REQUEST_NOT_FOUND); + assertTrue(request.getTransaction().hasBegin()); + assertTrue(request.getTransaction().getBegin().hasReadWrite()); + assertTrue(request.getLastStatement()); + assertEquals(0, statement.executeLargeUpdate(DDL, Statement.NO_GENERATED_KEYS)); assertEquals(0, statement.executeLargeUpdate(clientSideUpdate, Statement.NO_GENERATED_KEYS)); assertEquals( @@ -871,14 +940,13 @@ public void testExecuteAutoBatchDml() throws SQLException { assertEquals(3, request.getStatementsCount()); assertEquals(1, mockSpanner.countRequestsOfType(CommitRequest.class)); } - + @Test public void testLastStatement_AutoCommit_Query() throws SQLException { try (Connection connection = createJdbcConnection(); Statement statement = connection.createStatement()) { //noinspection EmptyTryBlock - try (ResultSet ignore = statement.executeQuery(query)) { - } + try (ResultSet ignore = statement.executeQuery(query)) {} } assertEquals(1, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); assertFalse(mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0).getLastStatement()); @@ -899,8 +967,7 @@ public void testLastStatement_AutoCommit_DmlReturning() throws SQLException { try (Connection connection = createJdbcConnection(); Statement statement = connection.createStatement()) { //noinspection EmptyTryBlock - try (ResultSet ignore = statement.executeQuery(dmlReturning)) { - } + try (ResultSet ignore = statement.executeQuery(dmlReturning)) {} } assertEquals(1, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); assertTrue(mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0).getLastStatement()); @@ -915,7 +982,8 @@ public void testLastStatement_AutoCommit_BatchDml() throws SQLException { statement.executeBatch(); } assertEquals(1, mockSpanner.countRequestsOfType(ExecuteBatchDmlRequest.class)); - assertTrue(mockSpanner.getRequestsOfType(ExecuteBatchDmlRequest.class).get(0).getLastStatements()); + assertTrue( + mockSpanner.getRequestsOfType(ExecuteBatchDmlRequest.class).get(0).getLastStatements()); } @Test @@ -924,8 +992,7 @@ public void testLastStatement_Transaction_Query() throws SQLException { Statement statement = connection.createStatement()) { connection.setAutoCommit(false); //noinspection EmptyTryBlock - try (ResultSet ignore = statement.executeQuery(query)) { - } + try (ResultSet ignore = statement.executeQuery(query)) {} connection.commit(); } assertEquals(1, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); @@ -950,8 +1017,7 @@ public void testLastStatement_Transaction_DmlReturning() throws SQLException { Statement statement = connection.createStatement()) { connection.setAutoCommit(false); //noinspection EmptyTryBlock - try (ResultSet ignore = statement.executeQuery(dmlReturning)) { - } + try (ResultSet ignore = statement.executeQuery(dmlReturning)) {} connection.commit(); } assertEquals(1, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); @@ -969,6 +1035,7 @@ public void testLastStatement_Transaction_BatchDml() throws SQLException { connection.commit(); } assertEquals(1, mockSpanner.countRequestsOfType(ExecuteBatchDmlRequest.class)); - assertFalse(mockSpanner.getRequestsOfType(ExecuteBatchDmlRequest.class).get(0).getLastStatements()); + assertFalse( + mockSpanner.getRequestsOfType(ExecuteBatchDmlRequest.class).get(0).getLastStatements()); } }