Skip to content

Commit a239225

Browse files
committed
Apply F3/F10 fixes to resetForNewExecution close path, clarify PR databricks#1422 provenance
- Narrow catch to SQLException|RuntimeException and upgrade to WARN with throwable in resetForNewExecution close path (consistency with closeServerOperation) - Update comment to clarify this code is from PR databricks#1422 and add skip condition (3) for serverOperationClosed Co-authored-by: Isaac Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
1 parent bbcf049 commit a239225

1 file changed

Lines changed: 11 additions & 17 deletions

File tree

src/main/java/com/databricks/jdbc/api/impl/DatabricksStatement.java

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,34 +1003,28 @@ private void resetForNewExecution() {
10031003
noMoreResults = false;
10041004
updateCount = -1;
10051005

1006-
// Close the previous server-side operation if it exists. This prevents resource
1007-
// leaks when a Statement is re-executed (e.g., PreparedStatement in a loop).
1008-
// This matches the behavior of pgJDBC, MySQL Connector/J, Trino JDBC, and
1009-
// Databricks Python SQL Connector — all close the previous operation on re-execute.
1006+
// Close the previous server-side operation if still open. This prevents resource
1007+
// leaks when a Statement is re-executed without closing the previous ResultSet.
1008+
// If the user closed the ResultSet before re-executing (best practice), the
1009+
// proactive close already set serverOperationClosed=true and this is a no-op.
10101010
//
1011-
// Note on directResultsReceived: we check the flag value from the PREVIOUS execution
1012-
// here. The flag is reset to false below, after this close attempt.
1013-
//
1014-
// Note on latency: this close is synchronous (adds one RPC round-trip before the next
1015-
// execution). This is consistent with pgJDBC's closeForNextExecution() which is also
1016-
// synchronous. The correctness benefit (no orphaned server operations) outweighs the
1017-
// latency cost for typical usage patterns.
1018-
//
1019-
// Skip if: (1) no previous execution (statementId==null), or
1020-
// (2) server already closed the operation (direct results).
1011+
// Skip if: (1) no previous execution (statementId==null),
1012+
// (2) server already closed the operation (direct/inline results), or
1013+
// (3) client already proactively closed it (via ResultSet.close()).
10211014
if (statementId != null && !directResultsReceived && !serverOperationClosed) {
10221015
try {
10231016
connection.getSession().getDatabricksClient().closeStatement(statementId);
10241017
serverOperationClosed = true;
1025-
} catch (Exception e) {
1018+
} catch (SQLException | RuntimeException e) {
10261019
// Don't block re-execution if closing the previous operation fails.
10271020
// This covers: network errors, operation already expired/evicted on server,
10281021
// and transport-level errors (e.g., unexpected server responses).
10291022
// The new execution will create a fresh operation with a new statementId.
1030-
LOGGER.debug(
1023+
LOGGER.warn(
10311024
"Failed to close previous server operation {} during re-execution: {}",
10321025
statementId,
1033-
e.getMessage());
1026+
e.getMessage(),
1027+
e);
10341028
}
10351029
}
10361030

0 commit comments

Comments
 (0)