Skip to content

Commit 394e3f9

Browse files
staticlibsstaticlibs
committed
Close pending statements on connection close
When connection is closed without closing its statements, the JDBC spec does NOT require to close such statements automatically. But pending statements are causing problems (see #101) and having them auto-closed seems to be a reasonable expectation from a client point of view. This change adds tracking and auto-closing of statements on connection close. It also adds/improves synchronizaton on closing connections/ statements/result sets. Straighforward sync on current object is used deliberately instead of other concurrency methods to keep the synchronization impl as simple as possible. Testing: new tests are added to check auto-closing of result sets and statements. Fixes: #101
1 parent fcd50cd commit 394e3f9

File tree

5 files changed

+194
-77
lines changed

5 files changed

+194
-77
lines changed

‎src/main/java/org/duckdb/DuckDBConnection.java

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@
2020
import java.sql.Savepoint;
2121
import java.sql.Statement;
2222
import java.sql.Struct;
23-
import java.util.Map;
24-
import java.util.Properties;
23+
import java.util.*;
2524
import java.util.concurrent.Executor;
2625
import org.duckdb.user.DuckDBMap;
2726
import org.duckdb.user.DuckDBUserArray;
@@ -37,6 +36,9 @@ public final class DuckDBConnection implements java.sql.Connection {
3736
boolean transactionRunning;
3837
final String url;
3938
private final boolean readOnly;
39+
// Tracking set for statements created from this connection,
40+
// must only be used from outside with track/untrack accessors.
41+
private final Set<DuckDBPreparedStatement> statements = new LinkedHashSet<>();
4042

4143
public static DuckDBConnection newConnection(String url, boolean readOnly, Properties properties)
4244
throws SQLException {
@@ -110,15 +112,32 @@ protected void finalize() throws Throwable {
110112
close();
111113
}
112114

113-
public synchronized void close() throws SQLException {
114-
if (conn_ref != null) {
115+
public void close() throws SQLException {
116+
if (conn_ref == null) {
117+
return;
118+
}
119+
synchronized (this) {
120+
if (conn_ref == null) {
121+
return;
122+
}
123+
// Closing remaining statements is not required by JDBC spec,
124+
// but it is a reasonable expectation from clients point of view.
125+
List<DuckDBPreparedStatement> stmtList = new ArrayList<>(statements);
126+
for (DuckDBPreparedStatement stmt : stmtList) {
127+
stmt.close();
128+
}
115129
DuckDBNative.duckdb_jdbc_disconnect(conn_ref);
116130
conn_ref = null;
117131
}
118132
}
119133

120134
public boolean isClosed() throws SQLException {
121-
return conn_ref == null;
135+
if (conn_ref == null) {
136+
return true;
137+
}
138+
synchronized (this) {
139+
return conn_ref == null;
140+
}
122141
}
123142

124143
public boolean isValid(int timeout) throws SQLException {
@@ -371,4 +390,19 @@ public void registerArrowStream(String name, Object arrow_array_stream) {
371390
long array_stream_address = getArrowStreamAddress(arrow_array_stream);
372391
DuckDBNative.duckdb_jdbc_arrow_register(conn_ref, array_stream_address, name.getBytes(StandardCharsets.UTF_8));
373392
}
393+
394+
void trackStatement(DuckDBPreparedStatement stmt) throws SQLException {
395+
synchronized (this) {
396+
if (isClosed()) {
397+
throw new SQLException("Connection was closed");
398+
}
399+
this.statements.add(stmt);
400+
}
401+
}
402+
403+
void untrackStatement(DuckDBPreparedStatement stmt) throws SQLException {
404+
synchronized (this) {
405+
this.statements.remove(stmt);
406+
}
407+
}
374408
}

‎src/main/java/org/duckdb/DuckDBDatabaseMetaData.java

Lines changed: 47 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -171,74 +171,74 @@ public String getIdentifierQuoteString() throws SQLException {
171171

172172
@Override
173173
public String getSQLKeywords() throws SQLException {
174-
Statement statement = conn.createStatement();
175-
statement.closeOnCompletion();
176-
ResultSet rs = statement.executeQuery("SELECT keyword_name FROM duckdb_keywords()");
177-
StringBuilder sb = new StringBuilder();
178-
while (rs.next()) {
179-
sb.append(rs.getString(1));
180-
sb.append(',');
174+
try (Statement statement = conn.createStatement();
175+
ResultSet rs = statement.executeQuery("SELECT keyword_name FROM duckdb_keywords()")) {
176+
StringBuilder sb = new StringBuilder();
177+
while (rs.next()) {
178+
sb.append(rs.getString(1));
179+
sb.append(',');
180+
}
181+
return sb.toString();
181182
}
182-
return sb.toString();
183183
}
184184

185185
@Override
186186
public String getNumericFunctions() throws SQLException {
187-
Statement statement = conn.createStatement();
188-
statement.closeOnCompletion();
189-
ResultSet rs = statement.executeQuery("SELECT DISTINCT function_name FROM duckdb_functions() "
190-
+ "WHERE parameter_types[1] ='DECIMAL'"
191-
+ "OR parameter_types[1] ='DOUBLE'"
192-
+ "OR parameter_types[1] ='SMALLINT'"
193-
+ "OR parameter_types[1] = 'BIGINT'");
194-
StringBuilder sb = new StringBuilder();
195-
while (rs.next()) {
196-
sb.append(rs.getString(1));
197-
sb.append(',');
187+
try (Statement statement = conn.createStatement();
188+
ResultSet rs = statement.executeQuery("SELECT DISTINCT function_name FROM duckdb_functions() "
189+
+ "WHERE parameter_types[1] ='DECIMAL'"
190+
+ "OR parameter_types[1] ='DOUBLE'"
191+
+ "OR parameter_types[1] ='SMALLINT'"
192+
+ "OR parameter_types[1] = 'BIGINT'")) {
193+
StringBuilder sb = new StringBuilder();
194+
while (rs.next()) {
195+
sb.append(rs.getString(1));
196+
sb.append(',');
197+
}
198+
return sb.toString();
198199
}
199-
return sb.toString();
200200
}
201201

202202
@Override
203203
public String getStringFunctions() throws SQLException {
204-
Statement statement = conn.createStatement();
205-
statement.closeOnCompletion();
206-
ResultSet rs = statement.executeQuery(
207-
"SELECT DISTINCT function_name FROM duckdb_functions() WHERE parameter_types[1] = 'VARCHAR'");
208-
StringBuilder sb = new StringBuilder();
209-
while (rs.next()) {
210-
sb.append(rs.getString(1));
211-
sb.append(',');
204+
try (Statement statement = conn.createStatement();
205+
ResultSet rs = statement.executeQuery(
206+
"SELECT DISTINCT function_name FROM duckdb_functions() WHERE parameter_types[1] = 'VARCHAR'")) {
207+
StringBuilder sb = new StringBuilder();
208+
while (rs.next()) {
209+
sb.append(rs.getString(1));
210+
sb.append(',');
211+
}
212+
return sb.toString();
212213
}
213-
return sb.toString();
214214
}
215215

216216
@Override
217217
public String getSystemFunctions() throws SQLException {
218-
Statement statement = conn.createStatement();
219-
statement.closeOnCompletion();
220-
ResultSet rs = statement.executeQuery(
221-
"SELECT DISTINCT function_name FROM duckdb_functions() WHERE length(parameter_types) = 0");
222-
StringBuilder sb = new StringBuilder();
223-
while (rs.next()) {
224-
sb.append(rs.getString(1));
225-
sb.append(',');
218+
try (Statement statement = conn.createStatement();
219+
ResultSet rs = statement.executeQuery(
220+
"SELECT DISTINCT function_name FROM duckdb_functions() WHERE length(parameter_types) = 0")) {
221+
StringBuilder sb = new StringBuilder();
222+
while (rs.next()) {
223+
sb.append(rs.getString(1));
224+
sb.append(',');
225+
}
226+
return sb.toString();
226227
}
227-
return sb.toString();
228228
}
229229

230230
@Override
231231
public String getTimeDateFunctions() throws SQLException {
232-
Statement statement = conn.createStatement();
233-
statement.closeOnCompletion();
234-
ResultSet rs = statement.executeQuery(
235-
"SELECT DISTINCT function_name FROM duckdb_functions() WHERE parameter_types[1] LIKE 'TIME%'");
236-
StringBuilder sb = new StringBuilder();
237-
while (rs.next()) {
238-
sb.append(rs.getString(1));
239-
sb.append(',');
232+
try (Statement statement = conn.createStatement();
233+
ResultSet rs = statement.executeQuery(
234+
"SELECT DISTINCT function_name FROM duckdb_functions() WHERE parameter_types[1] LIKE 'TIME%'")) {
235+
StringBuilder sb = new StringBuilder();
236+
while (rs.next()) {
237+
sb.append(rs.getString(1));
238+
sb.append(',');
239+
}
240+
return sb.toString();
240241
}
241-
return sb.toString();
242242
}
243243

244244
@Override

‎src/main/java/org/duckdb/DuckDBPreparedStatement.java

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package org.duckdb;
22

3-
import java.io.ByteArrayOutputStream;
43
import java.io.IOException;
54
import java.io.InputStream;
65
import java.io.Reader;
@@ -31,7 +30,6 @@
3130
import java.time.LocalDateTime;
3231
import java.time.OffsetDateTime;
3332
import java.util.ArrayList;
34-
import java.util.Arrays;
3533
import java.util.Calendar;
3634
import java.util.List;
3735
import java.util.logging.Level;
@@ -49,6 +47,7 @@ public class DuckDBPreparedStatement implements PreparedStatement {
4947
private boolean returnsNothing = false;
5048
private boolean returnsResultSet = false;
5149
boolean closeOnCompletion = false;
50+
boolean closing = false;
5251
private Object[] params = new Object[0];
5352
private DuckDBResultSetMetaData meta = null;
5453
private final List<Object[]> batchedParams = new ArrayList<>();
@@ -61,16 +60,14 @@ public DuckDBPreparedStatement(DuckDBConnection conn) throws SQLException {
6160
throw new SQLException("connection parameter cannot be null");
6261
}
6362
this.conn = conn;
63+
this.conn.trackStatement(this);
6464
}
6565

6666
public DuckDBPreparedStatement(DuckDBConnection conn, String sql) throws SQLException {
67-
if (conn == null) {
68-
throw new SQLException("connection parameter cannot be null");
69-
}
67+
this(conn);
7068
if (sql == null) {
7169
throw new SQLException("sql query parameter cannot be null");
7270
}
73-
this.conn = conn;
7471
this.isPreparedStatement = true;
7572
prepare(sql);
7673
}
@@ -299,15 +296,25 @@ public void clearParameters() throws SQLException {
299296

300297
@Override
301298
public void close() throws SQLException {
302-
if (select_result != null) {
303-
select_result.close();
304-
select_result = null;
299+
if (conn == null) {
300+
return;
305301
}
306-
if (stmt_ref != null) {
307-
DuckDBNative.duckdb_jdbc_release(stmt_ref);
308-
stmt_ref = null;
302+
synchronized (this) {
303+
if (conn == null) {
304+
return;
305+
}
306+
closing = true;
307+
if (select_result != null) {
308+
select_result.close();
309+
select_result = null;
310+
}
311+
if (stmt_ref != null) {
312+
DuckDBNative.duckdb_jdbc_release(stmt_ref);
313+
stmt_ref = null;
314+
}
315+
conn.untrackStatement(this);
316+
conn = null; // we use this as a check for closed-ness
309317
}
310-
conn = null; // we use this as a check for closed-ness
311318
}
312319

313320
protected void finalize() throws Throwable {
@@ -352,9 +359,14 @@ public void setQueryTimeout(int seconds) throws SQLException {
352359
* It is not safe to call this function when the connection is already closed.
353360
*/
354361
@Override
355-
public synchronized void cancel() throws SQLException {
356-
if (conn.conn_ref != null) {
357-
DuckDBNative.duckdb_jdbc_interrupt(conn.conn_ref);
362+
public void cancel() throws SQLException {
363+
synchronized (this) {
364+
if (isClosed()) {
365+
throw new SQLException("Statement was closed");
366+
}
367+
if (conn.conn_ref != null) {
368+
DuckDBNative.duckdb_jdbc_interrupt(conn.conn_ref);
369+
}
358370
}
359371
}
360372

@@ -559,7 +571,12 @@ public int getResultSetHoldability() throws SQLException {
559571

560572
@Override
561573
public boolean isClosed() throws SQLException {
562-
return conn == null;
574+
if (conn == null) {
575+
return true;
576+
}
577+
synchronized (this) {
578+
return conn == null;
579+
}
563580
}
564581

565582
@Override
@@ -576,7 +593,9 @@ public boolean isPoolable() throws SQLException {
576593
public void closeOnCompletion() throws SQLException {
577594
if (isClosed())
578595
throw new SQLException("Statement is closed");
579-
closeOnCompletion = true;
596+
synchronized (this) {
597+
closeOnCompletion = true;
598+
}
580599
}
581600

582601
@Override

‎src/main/java/org/duckdb/DuckDBResultSet.java

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import java.time.LocalDateTime;
3232
import java.time.OffsetDateTime;
3333
import java.time.OffsetTime;
34-
import java.util.Arrays;
3534
import java.util.Calendar;
3635
import java.util.Map;
3736
import java.util.Objects;
@@ -92,15 +91,22 @@ public synchronized boolean next() throws SQLException {
9291
return true;
9392
}
9493

95-
public synchronized void close() throws SQLException {
96-
if (result_ref != null) {
94+
public void close() throws SQLException {
95+
if (result_ref == null) {
96+
return;
97+
}
98+
synchronized (this) {
99+
if (result_ref == null) {
100+
return;
101+
}
97102
DuckDBNative.duckdb_jdbc_free_result(result_ref);
98103
// Nullness is used to determine whether we're closed
99104
result_ref = null;
100-
105+
}
106+
synchronized (stmt) {
101107
// isCloseOnCompletion() throws if already closed, and we can't check for isClosed() because it could change
102108
// between when we check and call isCloseOnCompletion, so access the field directly.
103-
if (stmt.closeOnCompletion) {
109+
if (stmt.closeOnCompletion && !stmt.closing) {
104110
stmt.close();
105111
}
106112
}
@@ -110,8 +116,13 @@ protected void finalize() throws Throwable {
110116
close();
111117
}
112118

113-
public synchronized boolean isClosed() throws SQLException {
114-
return result_ref == null;
119+
public boolean isClosed() throws SQLException {
120+
if (result_ref == null) {
121+
return true;
122+
}
123+
synchronized (this) {
124+
return result_ref == null;
125+
}
115126
}
116127

117128
private void check(int columnIndex) throws SQLException {

0 commit comments

Comments
 (0)