Skip to content

Commit 5f76129

Browse files
fix: trust terminal async row counts
Signed-off-by: Aakash Saravanan <aakash.saravanan@databricks.com>
1 parent 0b93e03 commit 5f76129

2 files changed

Lines changed: 37 additions & 9 deletions

File tree

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -934,8 +934,9 @@ def get_execution_result(
934934
"""Fetch a completed async result and its latest operation metadata.
935935
936936
FetchResults does not carry ``numModifiedRows``, so reuse a terminal
937-
status observed by ``get_query_state`` or refresh it when the caller
938-
resumes directly by command ID.
937+
status observed for the same ``CommandId`` instance by
938+
``get_query_state``, or refresh the status when the caller resumes
939+
directly by command ID.
939940
"""
940941
thrift_handle = command_id.to_thrift_handle()
941942
if not thrift_handle:
@@ -949,6 +950,12 @@ def get_execution_result(
949950
thrift_handle, operation_status
950951
)
951952

953+
num_modified_rows = (
954+
operation_status.numModifiedRows
955+
if operation_status.operationState == ttypes.TOperationState.FINISHED_STATE
956+
else None
957+
)
958+
952959
req = ttypes.TFetchResultsReq(
953960
operationHandle=ttypes.TOperationHandle(
954961
thrift_handle.operationId,
@@ -999,7 +1006,7 @@ def get_execution_result(
9991006
is_staging_operation=is_staging_operation,
10001007
arrow_schema_bytes=schema_bytes,
10011008
result_format=t_result_set_metadata_resp.resultFormat,
1002-
num_modified_rows=operation_status.numModifiedRows,
1009+
num_modified_rows=num_modified_rows,
10031010
)
10041011

10051012
return ThriftResultSet(

‎tests/unit/test_thrift_backend.py‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,19 @@ def test_metadata_catalog_escaping(
135135

136136

137137
@pytest.mark.skipif(pyarrow is None, reason="PyArrow is not installed")
138-
@pytest.mark.parametrize("count", [7, 0, None])
138+
@pytest.mark.parametrize(
139+
"operation_state,count,expected_count",
140+
[
141+
(ttypes.TOperationState.FINISHED_STATE, 7, 7),
142+
(ttypes.TOperationState.FINISHED_STATE, 0, 0),
143+
(ttypes.TOperationState.FINISHED_STATE, None, None),
144+
(ttypes.TOperationState.RUNNING_STATE, 7, None),
145+
],
146+
)
139147
@pytest.mark.parametrize("cache_status", [True, False])
140-
def test_async_result_reads_status_count_and_preserves_rows(count, cache_status):
148+
def test_async_result_reads_only_terminal_status_count_and_preserves_rows(
149+
operation_state, count, expected_count, cache_status
150+
):
141151
with (
142152
patch(
143153
"databricks.sql.backend.thrift_backend.TCLIService.Client", autospec=True
@@ -155,7 +165,7 @@ def test_async_result_reads_status_count_and_preserves_rows(count, cache_status)
155165
)
156166
operation_status = ttypes.TGetOperationStatusResp(
157167
status=ThriftBackendTestSuite.okay_status,
158-
operationState=ttypes.TOperationState.FINISHED_STATE,
168+
operationState=operation_state,
159169
numModifiedRows=count,
160170
)
161171
backend._poll_for_status = Mock(return_value=operation_status)
@@ -173,12 +183,23 @@ def test_async_result_reads_status_count_and_preserves_rows(count, cache_status)
173183
)
174184
cid = CommandId.from_thrift_handle(ThriftBackendTestSuite.operation_handle)
175185
if cache_status:
176-
assert backend.get_query_state(cid) == CommandState.SUCCEEDED
186+
expected_state = (
187+
CommandState.SUCCEEDED
188+
if operation_state == ttypes.TOperationState.FINISHED_STATE
189+
else CommandState.RUNNING
190+
)
191+
assert backend.get_query_state(cid) == expected_state
177192
backend.get_execution_result(cid, Mock())
178193

179-
backend._poll_for_status.assert_called_once_with(cid.to_thrift_handle())
194+
expected_polls = (
195+
2
196+
if cache_status and operation_state == ttypes.TOperationState.RUNNING_STATE
197+
else 1
198+
)
199+
assert backend._poll_for_status.call_count == expected_polls
200+
backend._poll_for_status.assert_called_with(cid.to_thrift_handle())
180201
response = result_set.call_args.kwargs["execute_response"]
181-
assert response.num_modified_rows == count
202+
assert response.num_modified_rows == expected_count
182203
assert result_set.call_args.kwargs["t_row_set"] is sentinel.counter_rows
183204

184205

0 commit comments

Comments
 (0)