Skip to content

Commit

Permalink
fix: release ResultSet on Statement#close() (#1567)
Browse files Browse the repository at this point in the history
* fix: release ResultSet on Statement#close()

Release the underlying resources of any ResultSet when Statement#close()
is called. This is necessary in case someone executes a query in
auto-commit mode using Statement#execute(String), and never reads and/or
closes the result that was returned.

* fix: close result set when Statement is closed
  • Loading branch information
olavloite committed Apr 16, 2024
1 parent a71c4f5 commit 2258ae3
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ public void cancel() throws SQLException {
}

@Override
public void close() {
public void close() throws SQLException {
this.closed = true;
}

Expand Down
19 changes: 16 additions & 3 deletions src/main/java/com/google/cloud/spanner/jdbc/JdbcStatement.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ enum BatchType {
super(connection);
}

@Override
public void close() throws SQLException {
setCurrentResultSet(null);
super.close();
}

@Override
public ResultSet executeQuery(String sql) throws SQLException {
checkClosed();
Expand Down Expand Up @@ -267,19 +273,19 @@ boolean executeStatement(Statement statement, ImmutableList<String> generatedKey
// keys. We can safely use '==', as the addReturningToStatement(..) method returns the same
// instance if no generated keys were requested.
if (statementWithReturning == statement) {
currentResultSet = JdbcResultSet.of(this, result.getResultSet());
setCurrentResultSet(JdbcResultSet.of(this, result.getResultSet()));
currentUpdateCount = JdbcConstants.STATEMENT_RESULT_SET;
return true;
}
this.currentGeneratedKeys = JdbcResultSet.copyOf(result.getResultSet());
this.currentUpdateCount = extractUpdateCountAndClose(result.getResultSet());
return false;
case UPDATE_COUNT:
currentResultSet = null;
setCurrentResultSet(null);
currentUpdateCount = result.getUpdateCount();
return false;
case NO_RESULT:
currentResultSet = null;
setCurrentResultSet(null);
currentUpdateCount = JdbcConstants.STATEMENT_NO_RESULT;
return false;
default:
Expand All @@ -294,6 +300,13 @@ public ResultSet getResultSet() throws SQLException {
return currentResultSet;
}

void setCurrentResultSet(ResultSet resultSet) throws SQLException {
if (this.currentResultSet != null) {
this.currentResultSet.close();
}
this.currentResultSet = resultSet;
}

/**
* Returns the update count of the last update statement. Will return {@link
* JdbcConstants#STATEMENT_RESULT_SET} if the last statement returned a {@link ResultSet} and will
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* Copyright 2024 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.cloud.spanner.jdbc;

import com.google.cloud.spanner.connection.AbstractMockServerTest;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.sql.Statement;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public class StatementResourcesTest extends AbstractMockServerTest {
private static final int MIN_SESSIONS = 1;
private static final int MAX_SESSIONS = 2;

private String createUrl() {
return String.format(
"jdbc:cloudspanner://localhost:%d/projects/%s/instances/%s/databases/%s"
+ "?usePlainText=true;minSessions=%d;maxSessions=%d",
getPort(), "proj", "inst", "db", MIN_SESSIONS, MAX_SESSIONS);
}

@Test
public void testMultipleQueriesOnOneStatement() throws SQLException {
try (Connection connection = DriverManager.getConnection(createUrl())) {
try (Statement statement = connection.createStatement()) {
for (int i = 0; i < MAX_SESSIONS + 1; i++) {
// Execute a query without reading or closing the result.
statement.execute("SELECT 1");
}
}
}
}

@Test
public void testMultipleStatementsWithOneQuery() throws SQLException {
try (Connection connection = DriverManager.getConnection(createUrl())) {
for (int i = 0; i < MAX_SESSIONS + 1; i++) {
try (Statement statement = connection.createStatement()) {
// Execute a query without reading or closing the result.
statement.execute("SELECT 1");
}
}
}
}

@Test
public void testMultipleQueriesOnOnePreparedStatement() throws SQLException {
try (Connection connection = DriverManager.getConnection(createUrl())) {
try (PreparedStatement statement = connection.prepareStatement("SELECT 1")) {
for (int i = 0; i < MAX_SESSIONS + 1; i++) {
// Execute a query without reading or closing the result.
statement.execute();
}
}
}
}

@Test
public void testMultiplePreparedStatementsWithOneQuery() throws SQLException {
try (Connection connection = DriverManager.getConnection(createUrl())) {
for (int i = 0; i < MAX_SESSIONS + 1; i++) {
try (PreparedStatement statement = connection.prepareStatement("SELECT 1")) {
// Execute a query without reading or closing the result.
statement.execute();
}
}
}
}
}

0 comments on commit 2258ae3

Please sign in to comment.