GeezerVison

🟠 High — handle.num_modified_rows is read directly, but that attribute only exists in the companion binding (databricks-sql-kernel#432). The dependency floor is still ^1.4.0, and the PR description says the floor will be raised later. On any wheel the resolver currently allows, this line raises AttributeError. The surrounding except Exception then turns it into a DatabaseError. The result is that every get_async_execution_result() on use_kernel=True fails, including plain async SELECTs, which work today. It also leaks the result set _make_result_set already built, since that result set (and its stream) is never closed.

The sync path in this file already handles this safely with getattr(executed, "num_modified_rows", None) (line 695). Either do the same here so older wheels just report rowcount=-1, or bump the pyproject floor in this same PR. Consider also adding a unit test where the handle has no num_modified_rows attribute. The new parametrized test always sets it, so the old-wheel case is never exercised.

The connector release requires kernel #432 and a package minimum set to the version containing it, as stated in the PR description. That version update is pending publication of the companion binding; v1.4.0 is still the latest released kernel. The direct getter is verified against the pinned source build in the native tests and live matrix. Keeping this release prerequisite visible.

Understood. The kernel floor bump has to wait until the binding that includes #432 is published, and the native/live tests build against that source. Until then, though, line 970 is still a direct handle.num_modified_rows read, and pyproject still allows ^1.4.0. If this merges before the floor bump, every kernel async get_async_execution_result() raises AttributeError and leaks the result set. A one-line getattr(handle, "num_modified_rows", None), like the sync path uses, would make this safe whenever the two land. I'm leaving this open as a merge gate until the floor is raised in this PR or the guarded read goes in.

🟠 High — This reads handle.num_modified_rows directly. According to the PR description, that attribute on ExecutedAsyncStatement is added by kernel PR #432, which hasn't been released yet. pyproject.toml still allows databricks-sql-kernel ^1.4.0, and KERNEL_REV only affects source-built CI.

On a 1.4.x wheel, the attribute lookup raises AttributeError. The except Exception below then turns it into a DatabaseError. The result is that every get_async_execution_result() on use_kernel=True fails, including plain async SELECTs that worked before this change. The _make_result_set result is also discarded without being closed.

The sync path a few hundred lines up guards against this with getattr(executed, "num_modified_rows", None). Use the same pattern here, which degrades to rowcount=-1 on older wheels. Alternatively, bump the dependency floor in the same PR so the requirement is enforced at install time, not only in the PR text.

Kernel #432 is merged and KERNEL_REV points to its merge commit. The remaining dependency-version update requires publication of that kernel release and must land before this connector is merged or released. The source-built and native tests use that binding. Leaving this release prerequisite open.

Agreed, merging #432 and pinning KERNEL_REV to its merge commit means the source-built and native CI jobs exercise the new attribute. But pyproject.toml still allows a 1.4.x wheel, and line 970 still reads handle.num_modified_rows directly. Until the dependency floor goes up, any install that resolves to 1.4.x will fail every kernel get_async_execution_result(), including plain async SELECTs. I'm keeping this thread open as the merge gate you described, so it closes when the floor bump lands. If the release slips, the getattr(handle, "num_modified_rows", None) fallback the sync path already uses would remove the gate.

🟠 High — handle.num_modified_rows is read directly. The PR description says this attribute only exists in a kernel release that ships databricks-sql-kernel#432, but pyproject.toml still lets the resolver install ^1.4.0. On a 1.4.x wheel this line raises AttributeError. The surrounding except wraps it as a DatabaseError, so every get_async_execution_result() on the kernel path fails, including async SELECTs that worked before. The result set built just above it is also left unclosed. The sync path at line 695 already reads this attribute with getattr(..., None).

Fix: use getattr(handle, "num_modified_rows", None) so older wheels fall back to rowcount=-1, or bump the kernel floor in this same PR. Add a unit test where the handle has no num_modified_rows attribute.

🔵 Low — When resuming directly by command ID (no cached terminal status), get_execution_result now always sends an extra GetOperationStatus RPC before FetchResults. That changes behavior: a failed or closed operation now raises from the status check, where before the error came from the fetch. That is probably the intended fix. But the error type and message for those cases have no test beyond the single ERROR_STATE case, and nothing covers the CLOSED/CANCELED states. Consider adding a parametrized case for CLOSED_STATE and CANCELED_STATE so the new pre-fetch guard is pinned.