Skip to content

Commit

Permalink
#690 Do not close result set after fetching last row in auto-commit mode
Browse files Browse the repository at this point in the history
  • Loading branch information
mrotteveel committed Jun 21, 2024
1 parent f31067c commit f05e931
Show file tree
Hide file tree
Showing 14 changed files with 124 additions and 280 deletions.
24 changes: 24 additions & 0 deletions src/docs/asciidoc/release_notes.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,20 @@ If you find cases where you think Jaybird should not (or on the contrary should)

For more information, see also https://github.com/FirebirdSQL/jaybird/blob/master/devdoc/jdp/jdp-2024-02-create-database-through-jdbc-url.adoc[jdp-2024-02: Create database through JDBC URL^].

[#no-close-after-last]
=== `ResultSet` in auto-commit no longer closed after last row

In previous Jaybird versions, iterating over a forward-only result set in auto-commit mode would implicitly close the result set after the last row was fetched -- i.e. when `next()` returned `false`.
This behaviour complied with the JDBC 3.0 requirements, but in JDBC 4.0 this requirement was removed, but the behaviour is still allowed.

In Jaybird 6, this implicit close has been removed.
In auto-commit mode, a result set will now remain open until explicitly closed using `ResultSet.close()`, when any statement is executed, when the auto-commit mode is disabled, or by the close of the `Statement` or `Connection`.

As a result set close is an auto-commit boundary, this change may delay commit of the active transaction until another action on the connection.
If you relied on this implicit close for correctness of your application, you may need to add an explicit call to `ResultSet.close()` -- e.g. using try-with-resources.

For more information, see also https://github.com/FirebirdSQL/jaybird/blob/master/devdoc/jdp/jdp-2024-03-do-not-close-result-set-after-last-row-in-auto-commit.adoc[jdp-2024-03: Do not close result set after last row in auto-commit^].

// TODO add major changes

[#other-fixes-and-changes]
Expand Down Expand Up @@ -1225,6 +1239,11 @@ If overridden transaction mappings are used, and the default isolation level has
As a result, switching isolation levels will now also result in read-only transactions, even if the mapping of the other isolation level is defined with `isc_tpb_write`.
You will need to explicitly call `setReadOnly(false)`, or -- better yet -- do not override transaction mappings with a `isc_tpb_read`, but always use `isc_tpb_write`, and control read-only state only through `setReadOnly`.

[#compat-no-close-after-last]
=== `ResultSet` in auto-commit no longer closed after last row

For more information, see <<no-close-after-last>>.

// TODO Document compatibility issues

[#removal-of-classes-packages-and-methods-without-deprecation]
Expand Down Expand Up @@ -1615,6 +1634,11 @@ replacement is `new SQLExceptionChainBuilder().append(exception)`
* `FBManagedConnection`
** `setReadOnly(boolean)` was renamed to `setTpbReadOnly(boolean)` to reflect what it actually does
** `isReadOnly()` was renamed to `isTpbReadOnly()` to reflect what it actually does
* `FBObjectListener.FetcherListener`
** `fetcherClosed(FBFetcher)` was removed
** `allRowsFetched(FBFetcher)` was removed
* `FBObjectListener.ResultSetListener`
** `allRowsFetched(ResultSet)` was removed

[#breaking-changes-unlikely]
=== Unlikely breaking changes
Expand Down
2 changes: 1 addition & 1 deletion src/main/org/firebirdsql/gds/ng/FbStatement.java
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public interface FbStatement extends ExceptionListenable, AutoCloseable {
* </p>
*
* @param transactionEnd
* close is in response to a transaction end or another operation which will close the cursor
* close is in response to a transaction end or another operation which will implicitly close the cursor
*/
void closeCursor(boolean transactionEnd) throws SQLException;

Expand Down
129 changes: 54 additions & 75 deletions src/main/org/firebirdsql/jdbc/FBObjectListener.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,31 +39,21 @@ private FBObjectListener() {

public interface FetcherListener {

/**
* Notify listener that underlying fetcher is closed.
*
* @param fetcher
* fetcher that was closed.
*/
void fetcherClosed(FBFetcher fetcher) throws SQLException;

/**
* Notify listener that underlying fetcher fetched all rows.
*
* @param fetcher
* fetcher that fetched all rows.
*/
void allRowsFetched(FBFetcher fetcher) throws SQLException;

/**
* Notify listener that underlying row was changed.
* <p>
* The default implementation does nothing.
* </p>
*
* @param fetcher
* instance of {@link FBFetcher} that caused this event.
* instance of {@link FBFetcher} that caused this event
* @param newRow
* new row.
* new row
*/
void rowChanged(FBFetcher fetcher, RowValue newRow) throws SQLException;
default void rowChanged(FBFetcher fetcher, RowValue newRow) throws SQLException {
// do nothing
}

}

/**
Expand All @@ -73,47 +63,48 @@ public interface ResultSetListener {

/**
* Notify listener that result set was closed.
* <p>
* The default implementation does nothing.
* </p>
*
* @param rs
* result set that was closed.
*/
void resultSetClosed(ResultSet rs) throws SQLException;

/**
* Notify listener that all rows were fetched. This event is used in auto-commit case to tell the statement that
* it is completed.
*
* @param rs
* result set that was completed.
* result set that was closed
*/
void allRowsFetched(ResultSet rs) throws SQLException;
default void resultSetClosed(ResultSet rs) throws SQLException {
// do nothing
}

/**
* Notify listener that execution of some row updating operation started.
* <p>
* The default implementation does nothing.
* </p>
*
* @param updater
* instance of {@link FirebirdRowUpdater}
* @throws SQLException
* if something went wrong.
*/
void executionStarted(FirebirdRowUpdater updater) throws SQLException;
default void executionStarted(FirebirdRowUpdater updater) throws SQLException {
// do nothing
}

/**
* Notify listener that execution of some row updating operation is completed.
* <p>
* The default implementation does nothing.
* </p>
*
* @param updater
* instance of {@link FirebirdRowUpdater}.
* @throws SQLException
* if something went wrong.
* instance of {@link FirebirdRowUpdater}
*/
void executionCompleted(FirebirdRowUpdater updater, boolean success) throws SQLException;
default void executionCompleted(FirebirdRowUpdater updater, boolean success) throws SQLException {
// do nothing
}

}

/**
* Implementation of {@link org.firebirdsql.jdbc.FBObjectListener.ResultSetListener} that implements all methods as
* empty methods.
* Implementation of {@link org.firebirdsql.jdbc.FBObjectListener.ResultSetListener} that does nothing.
*/
@SuppressWarnings("java:S1186")
public static final class NoActionResultSetListener implements ResultSetListener {

private static final ResultSetListener INSTANCE = new NoActionResultSetListener();
Expand All @@ -125,18 +116,6 @@ public static ResultSetListener instance() {
private NoActionResultSetListener() {
}

@Override
public void resultSetClosed(ResultSet rs) { }

@Override
public void allRowsFetched(ResultSet rs) { }

@Override
public void executionStarted(FirebirdRowUpdater updater) { }

@Override
public void executionCompleted(FirebirdRowUpdater updater, boolean success) { }

}

/**
Expand All @@ -149,25 +128,23 @@ public interface StatementListener {
*
* @return instance of {@link FBConnection}
* @throws SQLException
* if something went wrong.
* if something went wrong
*/
FBConnection getConnection() throws SQLException;

/**
* Notify listener that statement execution is being started.
*
* @param stmt
* statement that is being executed.
* @throws SQLException
* if something went wrong.
* statement that is being executed
*/
void executionStarted(FBStatement stmt) throws SQLException;

/**
* Notify the listener that statement was closed.
*
* @param stmt
* statement that was closed.
* statement that was closed
*/
void statementClosed(FBStatement stmt) throws SQLException;

Expand All @@ -184,14 +161,12 @@ public interface StatementListener {
* Notify the listener that statement is completed and tell whether execution was successful or not.
*
* @param stmt
* statement that was completed.
* statement that was completed
* @param success
* {@code true} if completion was successful.
* @throws SQLException
* if an error occurred.
* {@code true} if completion was successful
*/
void statementCompleted(FBStatement stmt, boolean success)
throws SQLException;
void statementCompleted(FBStatement stmt, boolean success) throws SQLException;

}

/**
Expand All @@ -201,30 +176,39 @@ public interface BlobListener {

/**
* Notify listener that execution of some BLOB operation had been started.
* <p>
* The default implementation does nothing.
* </p>
*
* @param blob
* instance of {@link FirebirdBlob} that caused this event.
* instance of {@link FirebirdBlob} that caused this event
* @throws SQLException
* if something went wrong.
* if something went wrong
*/
void executionStarted(FirebirdBlob blob) throws SQLException;
default void executionStarted(FirebirdBlob blob) throws SQLException {
// do nothing
}

/**
* Notify listener that execution of some BLOB operation had been completed.
* <p>
* The default implementation does nothing.
* </p>
*
* @param blob
* instance of {@link FirebirdBlob} that caused this event.
* @throws SQLException
* if something went wrong.
*/
void executionCompleted(FirebirdBlob blob) throws SQLException;
default void executionCompleted(FirebirdBlob blob) throws SQLException {
// do nothing
}

}

/**
* Implementation of {@link org.firebirdsql.jdbc.FBObjectListener.BlobListener} that implements all methods as
* empty methods.
* Implementation of {@link org.firebirdsql.jdbc.FBObjectListener.BlobListener} that does nothing.
*/
@SuppressWarnings("java:S1186")
public static final class NoActionBlobListener implements BlobListener {

private static final BlobListener INSTANCE = new NoActionBlobListener();
Expand All @@ -236,11 +220,6 @@ public static BlobListener instance() {
private NoActionBlobListener() {
}

@Override
public void executionStarted(FirebirdBlob blob) { }

@Override
public void executionCompleted(FirebirdBlob blob) { }
}

}
10 changes: 0 additions & 10 deletions src/main/org/firebirdsql/jdbc/FBResultSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,6 @@ public class FBResultSet implements ResultSet, FirebirdResultSet, FBObjectListen
private final ResultSetBehavior behavior;
private int fetchDirection = ResultSet.FETCH_FORWARD;

@Override
public void allRowsFetched(FBFetcher fetcher) throws SQLException {
listener.allRowsFetched(this);
}

@Override
public void fetcherClosed(FBFetcher fetcher) throws SQLException {
// ignore, there nothing to do here
}

@Override
public void rowChanged(FBFetcher fetcher, RowValue newRow) throws SQLException {
this.row = newRow;
Expand Down
1 change: 0 additions & 1 deletion src/main/org/firebirdsql/jdbc/FBServerScrollFetcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,6 @@ public void close(CompletionReason completionReason) throws SQLException {
rows.clear();
rowsOffset = 0;
rows = Collections.emptyList();
fetcherListener.fetcherClosed(this);
}
}

Expand Down
28 changes: 4 additions & 24 deletions src/main/org/firebirdsql/jdbc/FBStatement.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ private final class RSListener implements FBObjectListener.ResultSetListener {
private boolean rowUpdaterSeparateTransaction;

/**
* Notify that result set was closed. This method cleans the result
* set reference, so that call to {@link #close()} method will not cause
* exception.
* Notify that result set was closed. This method cleans the result set reference, so that calls to
* {@link #close()} method will not cause an exception.
*
* @param rs result set that was closed.
* @param rs
* result set that was closed.
*/
@Override
public void resultSetClosed(ResultSet rs) throws SQLException {
Expand All @@ -123,26 +123,6 @@ public void resultSetClosed(ResultSet rs) throws SQLException {
}
}

@Override
public void allRowsFetched(ResultSet rs) throws SQLException {

/*
* According to the JDBC 3.0 specification (p.62) the result set
* is closed in the autocommit mode if one of the following occurs:
*
* - all of the rows have been retrieved
* - the associated Statement object is re-executed
* - another Statement object is executed on the same connection
*/

// according to the specification we close the result set and
// generate the "resultSetClosed" event, that in turn generates
// the "statementCompleted" event

if (connection != null && connection.getAutoCommit())
rs.close();
}

@Override
public void executionCompleted(FirebirdRowUpdater updater, boolean success) throws SQLException {
if (rowUpdaterSeparateTransaction) {
Expand Down
2 changes: 0 additions & 2 deletions src/main/org/firebirdsql/jdbc/FBStatementFetcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ public boolean next() throws SQLException {
} else if (getNextRow() == null || (maxRows != 0 && getRowNum() == maxRows)) {
setIsAfterLast(true);
allRowsFetched = true;
fetcherListener.allRowsFetched(this);
setRowNum(0);
return false;
} else {
Expand Down Expand Up @@ -222,7 +221,6 @@ public void close(CompletionReason completionReason) throws SQLException {
} finally {
stmt.removeStatementListener(rowListener);
rows = new ArrayDeque<>(0);
fetcherListener.fetcherClosed(this);
}
}

Expand Down
Loading

0 comments on commit f05e931

Please sign in to comment.