Skip to content

Commit 43a2c93

Browse files
fix: populate rowcount after async DML completion
Carry affected-row counts from kernel handles and Thrift operation status into async result sets, then publish them on the cursor. Preserve zero, unknown counts, and fetched counter rows. Signed-off-by: Aakash Saravanan <aakash.saravanan@databricks.com>
1 parent 559f1d3 commit 43a2c93

6 files changed

Lines changed: 123 additions & 2 deletions

File tree

‎src/databricks/sql/backend/kernel/client.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -966,7 +966,9 @@ def get_execution_result(
966966
# telemetry — an accepted, narrow gap limited to the case where
967967
# result-set construction fails after a successful await.
968968
try:
969-
return self._make_result_set(stream, cursor, command_id)
969+
result_set = self._make_result_set(stream, cursor, command_id)
970+
result_set.num_modified_rows = getattr(handle, "num_modified_rows", None)
971+
return result_set
970972
except Exception as exc:
971973
raise _wrap_kernel_exception("get_execution_result", exc) from exc
972974

‎src/databricks/sql/backend/thrift_backend.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,11 @@ def get_execution_result(
931931
if not thrift_handle:
932932
raise ValueError("Not a valid Thrift command ID")
933933

934+
operation_status = self._poll_for_status(thrift_handle)
935+
self._check_command_not_in_error_or_closed_state(
936+
thrift_handle, operation_status
937+
)
938+
934939
req = ttypes.TFetchResultsReq(
935940
operationHandle=ttypes.TOperationHandle(
936941
thrift_handle.operationId,
@@ -981,6 +986,7 @@ def get_execution_result(
981986
is_staging_operation=is_staging_operation,
982987
arrow_schema_bytes=schema_bytes,
983988
result_format=t_result_set_metadata_resp.resultFormat,
989+
num_modified_rows=operation_status.numModifiedRows,
984990
)
985991

986992
return ThriftResultSet(

‎src/databricks/sql/client.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1701,6 +1701,9 @@ def get_async_execution_result(self):
17011701
self.active_command_id, self
17021702
)
17031703

1704+
num_modified_rows = self.active_result_set.num_modified_rows
1705+
self.rowcount = num_modified_rows if num_modified_rows is not None else -1
1706+
17041707
if self.active_result_set and self.active_result_set.is_staging_operation:
17051708
self._handle_staging_operation(
17061709
staging_allowed_local_path=self.connection.staging_allowed_local_path

‎tests/unit/test_client.py‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,29 @@ def test_rowcount_resets_between_statements(self):
303303
cursor.execute("SELECT 1")
304304
self.assertEqual(cursor.rowcount, -1)
305305

306+
def test_async_rowcount_on_completion_and_reset(self):
307+
for count in (5, 0, None):
308+
with self.subTest(count=count):
309+
result_set = Mock(num_modified_rows=count, is_staging_operation=False)
310+
backend = ThriftDatabricksClientMockFactory.new()
311+
backend.get_execution_result.return_value = result_set
312+
backend.get_query_state.return_value = CommandState.SUCCEEDED
313+
cursor = client.Cursor(connection=Mock(), backend=backend)
314+
cursor.rowcount = 9
315+
316+
cursor.execute_async("UPDATE t SET x = 1")
317+
self.assertEqual(cursor.rowcount, -1)
318+
cursor.active_command_id = Mock(spec=CommandId)
319+
self.assertIs(cursor.get_async_execution_result(), cursor)
320+
self.assertEqual(cursor.rowcount, count if count is not None else -1)
321+
result_set.fetchall.assert_not_called()
322+
323+
result_set.num_modified_rows = None
324+
cursor.execute_async("SELECT 1")
325+
self.assertEqual(cursor.rowcount, -1)
326+
cursor.get_async_execution_result()
327+
self.assertEqual(cursor.rowcount, -1)
328+
306329
def test_closed_cursor_doesnt_allow_operations(self):
307330
cursor = client.Cursor(Mock(), Mock())
308331
cursor.close()

‎tests/unit/test_kernel_client.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1233,6 +1233,48 @@ def test_get_query_state_uses_reader_after_result_stream_started():
12331233
reader.status.assert_called_once_with()
12341234

12351235

1236+
@pytest.mark.parametrize("count", [7, 0, None, "unavailable"])
1237+
@pytest.mark.parametrize("source", ["owner", "reader", "attached"])
1238+
def test_async_result_count_after_await(source, count):
1239+
c = _make_client()
1240+
c._kernel_session = MagicMock()
1241+
cid = CommandId.from_sea_statement_id("async-count")
1242+
owner = MagicMock()
1243+
handle = owner
1244+
if source == "attached":
1245+
c._kernel_session.attach_async_statement.return_value = handle
1246+
else:
1247+
c._async_handles[cid.guid] = owner
1248+
if source == "reader":
1249+
c._async_result_stream_started.add(cid.guid)
1250+
handle = owner.reader.return_value
1251+
1252+
del handle.num_modified_rows
1253+
stream = MagicMock()
1254+
stream.arrow_schema.return_value = pa.schema([("num_affected_rows", pa.int64())])
1255+
stream.fetch_next_batch.side_effect = [
1256+
pa.record_batch(
1257+
[[count if isinstance(count, int) else 1]], names=["num_affected_rows"]
1258+
),
1259+
None,
1260+
]
1261+
1262+
def await_result():
1263+
if count != "unavailable":
1264+
handle.num_modified_rows = count
1265+
return stream
1266+
1267+
handle.await_result.side_effect = await_result
1268+
cursor = MagicMock(arraysize=100, buffer_size_bytes=1024)
1269+
rs = c.get_execution_result(cid, cursor)
1270+
1271+
assert rs.num_modified_rows == (None if count == "unavailable" else count)
1272+
stream.fetch_next_batch.assert_not_called()
1273+
assert [list(row) for row in rs.fetchall()] == [
1274+
[count if isinstance(count, int) else 1]
1275+
]
1276+
1277+
12361278
def test_get_execution_result_uses_retained_owning_handle_first():
12371279
"""The first in-process result fetch uses the retained submitting
12381280
handle so the kernel finalizes the original async statement telemetry."""

‎tests/unit/test_thrift_backend.py‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import itertools
44
import unittest
55
import pytest
6-
from unittest.mock import patch, MagicMock, Mock
6+
from unittest.mock import patch, MagicMock, Mock, sentinel
77
from ssl import CERT_NONE, CERT_REQUIRED
88
from urllib3 import HTTPSConnectionPool
99

@@ -134,6 +134,51 @@ def test_metadata_catalog_escaping(
134134
assert getattr(request, thrift_field) == value
135135

136136

137+
@pytest.mark.skipif(pyarrow is None, reason="PyArrow is not installed")
138+
@pytest.mark.parametrize("count", [7, 0, None])
139+
def test_async_result_reads_status_count_and_preserves_rows(count):
140+
with (
141+
patch(
142+
"databricks.sql.backend.thrift_backend.TCLIService.Client", autospec=True
143+
),
144+
patch("databricks.sql.backend.thrift_backend.ThriftResultSet") as result_set,
145+
):
146+
backend = ThriftDatabricksClient(
147+
"foobar",
148+
443,
149+
"path",
150+
[],
151+
auth_provider=AuthProvider(),
152+
ssl_options=SSLOptions(),
153+
http_client=MagicMock(),
154+
)
155+
operation_status = ttypes.TGetOperationStatusResp(
156+
status=ThriftBackendTestSuite.okay_status,
157+
operationState=ttypes.TOperationState.FINISHED_STATE,
158+
numModifiedRows=count,
159+
)
160+
backend._poll_for_status = Mock(return_value=operation_status)
161+
metadata = ttypes.TGetResultSetMetadataResp(
162+
schema=ttypes.TTableSchema(columns=[]),
163+
arrowSchema=pyarrow.schema([]).serialize().to_pybytes(),
164+
resultFormat=ttypes.TSparkRowSetType.ARROW_BASED_SET,
165+
)
166+
backend.make_request = Mock(
167+
return_value=Mock(
168+
resultSetMetadata=metadata,
169+
results=sentinel.counter_rows,
170+
hasMoreRows=False,
171+
)
172+
)
173+
cid = CommandId.from_thrift_handle(ThriftBackendTestSuite.operation_handle)
174+
backend.get_execution_result(cid, Mock())
175+
176+
backend._poll_for_status.assert_called_once_with(cid.to_thrift_handle())
177+
response = result_set.call_args.kwargs["execute_response"]
178+
assert response.num_modified_rows == count
179+
assert result_set.call_args.kwargs["t_row_set"] is sentinel.counter_rows
180+
181+
137182
@pytest.mark.skipif(pyarrow is None, reason="PyArrow is not installed")
138183
class ThriftBackendTestSuite(unittest.TestCase):
139184
okay_status = ttypes.TStatus(statusCode=ttypes.TStatusCode.SUCCESS_STATUS)

0 commit comments

Comments
 (0)