Cursor.close() leaks server-side handle for async commands that were never fetched (#791)

[peco-engineer-bot]

Summary

Automated fix for #791 — Cursor.close() leaks server-side handle for async commands that were never fetched.

Fixed Cursor.close() in src/databricks/sql/client.py to call backend.close_command(active_command_id) when active_result_set is None but a command id exists (the async-submit-without-fetch case), so the server-side handle no longer leaks; the call is wrapped in a try/except that logs a warning to stay safe across Thrift/SEA/kernel backends. Verified via the live E2E test and two unit tests.

Root cause & plan

Root cause: Cursor.close() in src/databricks/sql/client.py (lines ~1707-1712) sets self.open=False, nulls self.active_command_id, and only closes self.active_result_set via _close_and_clear_active_result_set(). For an async submission (execute_async / execute(..., async_op=True)) where get_async_execution_result() was never called, active_result_set is None, so backend.close_command(active_command_id) is never invoked. The server-side statement handle therefore leaks until the session (connection) is closed. This affects all three backends (Thrift, SEA, kernel) because the leak is in the shared Cursor.close() path, not backend-specific code. Each backend already implements a safe close_command (Thrift sends TCloseOperation, SEA sends DELETE/close, kernel no-ops on unknown ids).
Files: src/databricks/sql/client.py, tests/e2e/test_driver.py
Planned coverage:

• E2E (Thrift default backend): submit a long-running query with cursor.execute_async(); capture cursor.active_command_id and cursor.backend; assert the handle is live (get_query_state returns PENDING/RUNNING/SUCCEEDED); call cursor.close() without ever fetching; then assert the server no longer reports the handle as live — polling get_query_state on the same command id yields CLOSED/CANCELLED, or raises DatabaseError ('unexpectedly closed server side') which is treated as CLOSED. Fails today because close() never calls close_command, so the server still reports RUNNING/PENDING. (Async-submit-without-fetch handle leak on cursor.close() (shared close path; Thrift backend))

Files changed

• tests/e2e/test_driver.py
• src/databricks/sql/client.py
• tests/unit/test_client.py

Test plan

• tests/e2e/test_driver.py::TestPySQLAsyncQueriesSuite::test_close_frees_async_handle_without_fetch — fails (red) against the original code, passes (green) after the fix
• tests/unit/test_client.py::ClientTestSuite::test_close_frees_async_command_without_result_set — fails (red) against the original code, passes (green) after the fix
• tests/unit/test_client.py::ClientTestSuite::test_close_swallows_close_command_errors — fails (red) against the original code, passes (green) after the fix

🤖 Generated by engineer-bot (bug-fix flow) — review before merge.

[peco-review-bot]

Verdict: 1 Low

Looks good — the fix correctly closes the leaked server-side handle for async-submitted commands that were never fetched by issuing backend.close_command() in the new elif branch, guarded by try/except and reordering active_command_id = None to after the close. logger is defined in the module, all backends implement close_command(command_id), and there's no double-close risk since the branch only fires when active_result_set is None. One low-severity note on E2E coverage (Thrift-only) below.

[peco-review-bot]

✅ No issues identified by the review bot.

[eric-wang-1990]

We should never assign use_sea, there are only 2 active modes either default(thrift), or use_kernel which use kernel.