[PECOBLR-954] Close statements based on Execute Statement Response SEA

[madhav-db]

Description

Close statements based on Execute Statement Response SEA

Flow:

1. User calls executeQuery
2. executeInternal in DatabricksStatement is called, and creates direct same thread executor
3. Runs on same thread, calls getResultFromClient
4. After execution, server responds with status: CLOSED, we detect the state and call markAsClosed which soft closes the statement (does not shut down the executor)
5. We return result set to user
6. User calls statement.close()
7. DatabricksStatement sees isClosed=true, skips the server close (since it's already closed), closes result set and shuts down the executor.

Testing

Additional Notes to the Reviewer

[gopalldb]

Description

Close statements based on Execute Statement Response SEA

Testing

Additional Notes to the Reviewer

Add testing section

[Copilot]

Pull request overview

This PR implements server-side statement closure detection for Databricks JDBC connections. When the server returns a CLOSED status in the Execute Statement Response, the driver now soft-closes the statement locally without attempting to close it again on the server.

Key changes:

• Added markAsClosed() method to mark statements as closed when server indicates closure
• Modified statement close logic to skip server-side close calls for already-closed statements
• Added detection of CLOSED status in execute statement responses

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
DatabricksStatement.java	Added markAsClosed() method and updated close() to handle pre-closed statements
DatabricksSdkClient.java	Added detection of CLOSED status in execute responses to trigger markAsClosed()
DatabricksStatementTest.java	Added comprehensive test coverage for markAsClosed() scenarios
DatabricksSdkClientTest.java	Added tests for execute statement with closed status responses

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The early return path for isClosed doesn't shut down the executor or remove the statement from session. This creates inconsistent cleanup behavior compared to the normal close path. Consider calling shutdownExecutor() and handling session removal in this branch as well to ensure complete resource cleanup.

Suggested change

	}
	}
	if (removeFromSession) {
	this.connection.closeStatement(this);
	}
	shutDownExecutor();
	this.isClosed = true;
	return;

[Copilot]

When resultSet.close() throws an exception in the isClosed branch, the exception propagates without setting isClosed flag or logging. The normal close path has better error handling. Consider wrapping this in a try-catch to ensure the statement remains in a consistent state even if result set closure fails.

Suggested change

	this.resultSet.close();
	this.resultSet = null;
	try {
	this.resultSet.close();
	} catch (Exception e) {
	LOGGER.warn("Exception while closing resultSet in already closed statement", e);
	warnings = WarningUtil.addWarning(warnings, "Exception while closing resultSet: " + e.getMessage());
	} finally {
	this.resultSet = null;
	this.isClosed = true;
	}

[Copilot]

The markAsClosed() method is not synchronized, but it modifies shared state (isClosed) and calls methods that may have concurrency implications. If this method can be called from multiple threads (e.g., during statement execution and timeout handling), it could lead to race conditions. Consider adding synchronization or documenting thread-safety assumptions.