Skip to content

Commit 8a62208

Browse files
Support cursor.rowcount for DML on the Thrift backend (#784) (#847)
* Support cursor.rowcount for DML on the Thrift backend (#784) cursor.rowcount was hardcoded to -1 and never updated for the Thrift backend (the default). For DML (INSERT/UPDATE/DELETE/MERGE) the Databricks Thrift server reports the affected-row count in TGetOperationStatusResp.numModifiedRows, but the connector discarded it — _wait_until_command_done kept only operationState. Thread numModifiedRows through: _wait_until_command_done now returns the terminal status response, _handle_execute_response reads numModifiedRows from it, ExecuteResponse and ResultSet carry a num_modified_rows field, and Cursor.execute sets self.rowcount from it. rowcount resets to -1 before each statement so a DML count never leaks into a later SELECT. SELECT (and statements the server does not report a count for) leave rowcount at its -1 default. This brings the Thrift path in line with the kernel backend, which already surfaces num_modified_rows. Closes #784 Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * test(e2e): verify DML cursor.rowcount against a live warehouse (#784) End-to-end test in TestPySQLCoreSuite exercising INSERT/UPDATE/DELETE on a real Thrift warehouse and asserting cursor.rowcount reports the exact affected-row count (3/2/1), then that a following SELECT resets it to -1. Verified live on dogfood; fails with the fix reverted (rowcount stays -1). Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * Aggregate executemany rowcount across parameter sets (#784) Address PR review: executemany looped execute() per parameter set and, since each execute() resets rowcount, left rowcount equal to only the final set's affected-row count. Per PEP 249, rowcount after executemany should reflect the total across all operations. Accumulate the reported per-statement counts; if no statement reports a count (all SELECT / the server reports none), rowcount stays at its -1 default. Adds unit tests for the sum, mixed reported/unreported, and all-unreported cases, and extends the live e2e test to assert executemany aggregation. Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> --------- Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
1 parent 7e7b323 commit 8a62208

8 files changed

Lines changed: 326 additions & 24 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
# Unreleased
44
- Fix: `REMOVE` staging operations no longer require `staging_allowed_local_path` to be set, since removing a remote file does not touch the local filesystem (databricks/databricks-sql-python#726)
5+
- Report `cursor.rowcount` for DML on the Thrift backend: INSERT/UPDATE/DELETE/MERGE now set `rowcount` to the server's affected-row count instead of the hardcoded `-1`; SELECT (and statements the server does not report a count for) still return `-1`. `executemany` aggregates the count across all parameter sets per PEP 249 ([#784](https://github.com/databricks/databricks-sql-python/issues/784))
56

67
# 4.3.0 (2026-06-12)
78
- **New: optional Rust kernel backend (`use_kernel=True`).** Adds an alternative connection path backed by the native [`databricks-sql-kernel`](https://pypi.org/project/databricks-sql-kernel/) client (a Rust core exposed via PyO3), installable with the new `databricks-sql-connector[kernel]` extra. The kernel talks to Databricks over the **SEA (Statement Execution API) HTTP transport** — not Thrift — with CloudFetch and inline-Arrow result fetching, so `use_kernel=True` gives you a modern SEA-native client through the same DB-API surface. Supports PAT, OAuth M2M, and OAuth U2M auth. Requires Python >= 3.10 (the kernel wheel is `cp310-abi3`); on older interpreters the extra is a no-op and `use_kernel=True` raises a clear `ImportError`. The default backend remains Thrift — opt in per connection.

src/databricks/sql/backend/thrift_backend.py

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -801,7 +801,9 @@ def _hive_schema_to_description(t_table_schema, schema_bytes=None, host_url=None
801801
for col in t_table_schema.columns
802802
]
803803

804-
def _results_message_to_execute_response(self, resp, operation_state):
804+
def _results_message_to_execute_response(
805+
self, resp, operation_state, num_modified_rows=None
806+
):
805807
if resp.directResults and resp.directResults.resultSetMetadata:
806808
t_result_set_metadata_resp = resp.directResults.resultSetMetadata
807809
else:
@@ -864,6 +866,7 @@ def _results_message_to_execute_response(self, resp, operation_state):
864866
is_staging_operation=t_result_set_metadata_resp.isStagingOperation,
865867
arrow_schema_bytes=schema_bytes,
866868
result_format=t_result_set_metadata_resp.resultFormat,
869+
num_modified_rows=num_modified_rows,
867870
)
868871

869872
return execute_response, has_more_rows
@@ -945,6 +948,7 @@ def _wait_until_command_done(self, op_handle, initial_operation_status_resp):
945948
self._check_command_not_in_error_or_closed_state(
946949
op_handle, initial_operation_status_resp
947950
)
951+
final_status_resp = initial_operation_status_resp
948952
operation_state = (
949953
initial_operation_status_resp
950954
and initial_operation_status_resp.operationState
@@ -956,7 +960,10 @@ def _wait_until_command_done(self, op_handle, initial_operation_status_resp):
956960
poll_resp = self._poll_for_status(op_handle)
957961
operation_state = poll_resp.operationState
958962
self._check_command_not_in_error_or_closed_state(op_handle, poll_resp)
959-
return operation_state
963+
final_status_resp = poll_resp
964+
# Return the terminal status response (not just the state) so callers
965+
# can read ``numModifiedRows`` — the DML affected-row count — from it.
966+
return operation_state, final_status_resp
960967

961968
def get_query_state(self, command_id: CommandId) -> CommandState:
962969
thrift_handle = command_id.to_thrift_handle()
@@ -1274,12 +1281,21 @@ def _handle_execute_response(self, resp, cursor):
12741281
cursor.active_command_id = command_id
12751282
self._check_direct_results_for_error(resp.directResults, self._host)
12761283

1277-
final_operation_state = self._wait_until_command_done(
1284+
final_operation_state, final_status_resp = self._wait_until_command_done(
12781285
resp.operationHandle,
12791286
resp.directResults and resp.directResults.operationStatus,
12801287
)
12811288

1282-
return self._results_message_to_execute_response(resp, final_operation_state)
1289+
# ``numModifiedRows`` is populated by the server for DML statements
1290+
# (INSERT/UPDATE/DELETE/MERGE) and is None for SELECT. Surface it so it
1291+
# can flow to ``cursor.rowcount``.
1292+
num_modified_rows = (
1293+
final_status_resp.numModifiedRows if final_status_resp else None
1294+
)
1295+
1296+
return self._results_message_to_execute_response(
1297+
resp, final_operation_state, num_modified_rows
1298+
)
12831299

12841300
def _handle_execute_response_async(self, resp, cursor):
12851301
command_id = CommandId.from_thrift_handle(resp.operationHandle)

src/databricks/sql/backend/types.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,3 +425,7 @@ class ExecuteResponse:
425425
is_staging_operation: bool = False
426426
arrow_schema_bytes: Optional[bytes] = None
427427
result_format: Optional[Any] = None
428+
# Number of rows modified by a DML statement (INSERT/UPDATE/DELETE/MERGE),
429+
# surfaced as ``cursor.rowcount``. ``None`` for SELECT and any statement
430+
# for which the server does not report a count → ``rowcount`` stays at -1.
431+
num_modified_rows: Optional[int] = None

src/databricks/sql/client.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -874,7 +874,10 @@ def __init__(
874874

875875
self.connection: Connection = connection
876876

877-
self.rowcount: int = -1 # Return -1 as this is not supported
877+
# -1 until a statement runs. Set to the affected-row count after a DML
878+
# statement (INSERT/UPDATE/DELETE/MERGE); stays -1 for SELECT and any
879+
# statement the server does not report a modified-row count for.
880+
self.rowcount: int = -1
878881
self.buffer_size_bytes: int = result_buffer_size_bytes
879882
self.active_result_set: Union[ResultSet, None] = None
880883
self.arraysize: int = arraysize
@@ -1039,6 +1042,9 @@ def _close_and_clear_active_result_set(self):
10391042
self.active_result_set.close()
10401043
finally:
10411044
self.active_result_set = None
1045+
# Reset rowcount to its -1 default so a prior DML's count never
1046+
# leaks into a subsequent SELECT (or unreported) statement.
1047+
self.rowcount = -1
10421048

10431049
def _check_not_closed(self):
10441050
if not self.open:
@@ -1373,6 +1379,14 @@ def execute(
13731379
query_tags=query_tags,
13741380
)
13751381

1382+
# Surface the affected-row count for DML (INSERT/UPDATE/DELETE/MERGE) as
1383+
# cursor.rowcount instead of the hardcoded -1. num_modified_rows is None
1384+
# for SELECT (and statements the server does not report a count for) →
1385+
# leave rowcount at its -1 default.
1386+
num_modified_rows = getattr(self.active_result_set, "num_modified_rows", None)
1387+
if num_modified_rows is not None:
1388+
self.rowcount = num_modified_rows
1389+
13761390
if self.active_result_set and self.active_result_set.is_staging_operation:
13771391
self._handle_staging_operation(
13781392
staging_allowed_local_path=self.connection.staging_allowed_local_path,
@@ -1511,8 +1525,22 @@ def executemany(
15111525
15121526
:returns self
15131527
"""
1528+
# Per PEP 249, rowcount after executemany reflects the total rows
1529+
# affected across all parameter sets (or -1 when undeterminable). Each
1530+
# execute() resets self.rowcount and sets it from its own statement, so
1531+
# we accumulate the reported counts here. If no statement reports a
1532+
# count (e.g. all SELECT, or the server does not report one), rowcount
1533+
# stays at its -1 default.
1534+
total_rowcount = -1
15141535
for parameters in seq_of_parameters:
15151536
self.execute(operation, parameters, query_tags=query_tags)
1537+
if self.rowcount >= 0:
1538+
total_rowcount = (
1539+
self.rowcount
1540+
if total_rowcount < 0
1541+
else total_rowcount + self.rowcount
1542+
)
1543+
self.rowcount = total_rowcount
15161544
return self
15171545

15181546
@log_latency(StatementType.METADATA)

src/databricks/sql/result_set.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ def __init__(
5050
is_staging_operation: bool = False,
5151
lz4_compressed: bool = False,
5252
arrow_schema_bytes: Optional[bytes] = None,
53+
num_modified_rows: Optional[int] = None,
5354
):
5455
"""
5556
A ResultSet manages the results of a single command.
@@ -82,6 +83,8 @@ def __init__(
8283
self._is_staging_operation = is_staging_operation
8384
self.lz4_compressed = lz4_compressed
8485
self._arrow_schema_bytes = arrow_schema_bytes
86+
# Affected-row count for DML; None for SELECT / unreported.
87+
self.num_modified_rows = num_modified_rows
8588

8689
def __iter__(self):
8790
while True:
@@ -264,6 +267,7 @@ def __init__(
264267
is_staging_operation=execute_response.is_staging_operation,
265268
lz4_compressed=execute_response.lz4_compressed,
266269
arrow_schema_bytes=execute_response.arrow_schema_bytes,
270+
num_modified_rows=execute_response.num_modified_rows,
267271
)
268272

269273
# Initialize results queue if not provided

tests/e2e/test_driver.py

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@
6565
for name in test_loader.getTestCaseNames(DecimalTestsMixin):
6666
if name.startswith("test_"):
6767
fn = getattr(DecimalTestsMixin, name)
68-
decorated = skipUnless(pysql_supports_arrow(), "Decimal tests need arrow support")(
69-
fn
70-
)
68+
decorated = skipUnless(
69+
pysql_supports_arrow(), "Decimal tests need arrow support"
70+
)(fn)
7171
setattr(DecimalTestsMixin, name, decorated)
7272

7373

@@ -152,9 +152,7 @@ def test_query_with_large_wide_result_set(self, extra_params, lz4_compression):
152152
cursor.connection.lz4_compression = lz4_compression
153153
uuids = ", ".join(["uuid() uuid{}".format(i) for i in range(cols)])
154154
cursor.execute(
155-
"SELECT id, {uuids} FROM RANGE({rows})".format(
156-
uuids=uuids, rows=rows
157-
)
155+
"SELECT id, {uuids} FROM RANGE({rows})".format(uuids=uuids, rows=rows)
158156
)
159157
assert lz4_compression == cursor.active_result_set.lz4_compressed
160158
for row_id, row in enumerate(
@@ -455,6 +453,41 @@ def test_create_table_will_return_empty_result_set(self, extra_params):
455453
finally:
456454
cursor.execute("DROP TABLE IF EXISTS {}".format(table_name))
457455

456+
def test_dml_rowcount(self):
457+
"""cursor.rowcount reports the affected-row count for DML
458+
(INSERT/UPDATE/DELETE) instead of the hardcoded -1, and resets
459+
to -1 for a subsequent SELECT (GH #784)."""
460+
with self.cursor({}) as cursor:
461+
table_name = "table_{uuid}".format(uuid=str(uuid4()).replace("-", "_"))
462+
try:
463+
cursor.execute("CREATE TABLE {} (n INT)".format(table_name))
464+
465+
cursor.execute("INSERT INTO {} VALUES (1), (2), (3)".format(table_name))
466+
assert cursor.rowcount == 3, f"INSERT rowcount {cursor.rowcount!r}"
467+
468+
cursor.execute(
469+
"UPDATE {} SET n = n + 1 WHERE n >= 2".format(table_name)
470+
)
471+
assert cursor.rowcount == 2, f"UPDATE rowcount {cursor.rowcount!r}"
472+
473+
cursor.execute("DELETE FROM {} WHERE n = 1".format(table_name))
474+
assert cursor.rowcount == 1, f"DELETE rowcount {cursor.rowcount!r}"
475+
476+
# SELECT must reset rowcount to -1 (not leak the DELETE count).
477+
cursor.execute("SELECT * FROM {}".format(table_name))
478+
assert cursor.rowcount == -1, f"SELECT rowcount {cursor.rowcount!r}"
479+
assert len(cursor.fetchall()) == 2
480+
481+
# executemany aggregates the affected-row count across all
482+
# parameter sets (PEP 249), not just the last one.
483+
cursor.executemany(
484+
"INSERT INTO {} VALUES (%(v)s)".format(table_name),
485+
seq_of_parameters=[{"v": 10}, {"v": 20}, {"v": 30}],
486+
)
487+
assert cursor.rowcount == 3, f"executemany rowcount {cursor.rowcount!r}"
488+
finally:
489+
cursor.execute("DROP TABLE IF EXISTS {}".format(table_name))
490+
458491
def test_get_tables(self):
459492
with self.cursor({}) as cursor:
460493
table_name = "table_{uuid}".format(uuid=str(uuid4()).replace("-", "_"))
@@ -966,7 +999,13 @@ def test_timestamps_arrow(self):
966999
)
9671000
def test_multi_timestamps_arrow(self, extra_params):
9681001
with self.cursor(
969-
{"session_configuration": {"ansi_mode": False, "query_tags": "test:multi-timestamps,driver:python"}, **extra_params}
1002+
{
1003+
"session_configuration": {
1004+
"ansi_mode": False,
1005+
"query_tags": "test:multi-timestamps,driver:python",
1006+
},
1007+
**extra_params,
1008+
}
9701009
) as cursor:
9711010
query, expected = self.multi_query()
9721011
expected = [

tests/unit/test_client.py

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,56 @@ def test_executing_multiple_commands_uses_the_most_recent_command(self):
253253
mock_result_sets[0].fetchall.assert_not_called()
254254
mock_result_sets[1].fetchall.assert_called_once_with()
255255

256+
def test_rowcount_reports_num_modified_rows_for_dml(self):
257+
"""DML sets cursor.rowcount from the result set's num_modified_rows
258+
instead of the hardcoded -1 (GH #784)."""
259+
mock_result_set = Mock()
260+
mock_result_set.is_staging_operation = False
261+
mock_result_set.num_modified_rows = 5
262+
263+
mock_backend = ThriftDatabricksClientMockFactory.new()
264+
mock_backend.execute_command.return_value = mock_result_set
265+
266+
cursor = client.Cursor(connection=Mock(), backend=mock_backend)
267+
cursor.execute("UPDATE t SET x = 1 WHERE y = 2")
268+
269+
self.assertEqual(cursor.rowcount, 5)
270+
271+
def test_rowcount_stays_default_for_select(self):
272+
"""SELECT (num_modified_rows is None) leaves rowcount at -1."""
273+
mock_result_set = Mock()
274+
mock_result_set.is_staging_operation = False
275+
mock_result_set.num_modified_rows = None
276+
277+
mock_backend = ThriftDatabricksClientMockFactory.new()
278+
mock_backend.execute_command.return_value = mock_result_set
279+
280+
cursor = client.Cursor(connection=Mock(), backend=mock_backend)
281+
cursor.execute("SELECT 1")
282+
283+
self.assertEqual(cursor.rowcount, -1)
284+
285+
def test_rowcount_resets_between_statements(self):
286+
"""A DML count must not leak into a subsequent SELECT on the same
287+
cursor — rowcount resets to -1 before each execute."""
288+
dml_rs = Mock()
289+
dml_rs.is_staging_operation = False
290+
dml_rs.num_modified_rows = 7
291+
292+
select_rs = Mock()
293+
select_rs.is_staging_operation = False
294+
select_rs.num_modified_rows = None
295+
296+
mock_backend = ThriftDatabricksClientMockFactory.new()
297+
mock_backend.execute_command.side_effect = [dml_rs, select_rs]
298+
299+
cursor = client.Cursor(connection=Mock(), backend=mock_backend)
300+
cursor.execute("DELETE FROM t WHERE y = 2")
301+
self.assertEqual(cursor.rowcount, 7)
302+
303+
cursor.execute("SELECT 1")
304+
self.assertEqual(cursor.rowcount, -1)
305+
256306
def test_closed_cursor_doesnt_allow_operations(self):
257307
cursor = client.Cursor(Mock(), Mock())
258308
cursor.close()
@@ -450,6 +500,8 @@ def test_executemany_parameter_passhthrough_and_uses_last_result_set(self):
450500
# Set is_staging_operation to False to avoid _handle_staging_operation being called
451501
for mock_rs in mock_result_set_instances:
452502
mock_rs.is_staging_operation = False
503+
# SELECT statements: no modified-row count, so rowcount stays -1.
504+
mock_rs.num_modified_rows = None
453505

454506
mock_backend = ThriftDatabricksClientMockFactory.new()
455507
mock_backend.execute_command.side_effect = mock_result_set_instances
@@ -479,6 +531,63 @@ def test_executemany_parameter_passhthrough_and_uses_last_result_set(self):
479531
"last operation",
480532
)
481533

534+
def test_executemany_rowcount_sums_dml_across_parameter_sets(self):
535+
"""executemany aggregates rowcount across all parameter sets (PEP 249),
536+
rather than reporting only the final statement's count (GH #784)."""
537+
result_sets = []
538+
for n in (2, 3, 5):
539+
rs = Mock()
540+
rs.is_staging_operation = False
541+
rs.num_modified_rows = n
542+
result_sets.append(rs)
543+
544+
mock_backend = ThriftDatabricksClientMockFactory.new()
545+
mock_backend.execute_command.side_effect = result_sets
546+
547+
cursor = client.Cursor(Mock(), mock_backend)
548+
cursor.executemany(
549+
"INSERT INTO t VALUES (%(x)s)",
550+
seq_of_parameters=[{"x": 1}, {"x": 2}, {"x": 3}],
551+
)
552+
553+
self.assertEqual(cursor.rowcount, 10)
554+
555+
def test_executemany_rowcount_ignores_unreported_statements(self):
556+
"""Parameter sets the server doesn't report a count for (num_modified_rows
557+
None) don't drag the aggregate down; only reported counts are summed."""
558+
reported = Mock()
559+
reported.is_staging_operation = False
560+
reported.num_modified_rows = 4
561+
562+
unreported = Mock()
563+
unreported.is_staging_operation = False
564+
unreported.num_modified_rows = None
565+
566+
mock_backend = ThriftDatabricksClientMockFactory.new()
567+
mock_backend.execute_command.side_effect = [reported, unreported]
568+
569+
cursor = client.Cursor(Mock(), mock_backend)
570+
cursor.executemany("...", seq_of_parameters=[{"x": 1}, {"x": 2}])
571+
572+
self.assertEqual(cursor.rowcount, 4)
573+
574+
def test_executemany_rowcount_stays_default_when_nothing_reported(self):
575+
"""All-SELECT (or all-unreported) executemany leaves rowcount at -1."""
576+
result_sets = []
577+
for _ in range(2):
578+
rs = Mock()
579+
rs.is_staging_operation = False
580+
rs.num_modified_rows = None
581+
result_sets.append(rs)
582+
583+
mock_backend = ThriftDatabricksClientMockFactory.new()
584+
mock_backend.execute_command.side_effect = result_sets
585+
586+
cursor = client.Cursor(Mock(), mock_backend)
587+
cursor.executemany("SELECT %(x)s", seq_of_parameters=[{"x": 1}, {"x": 2}])
588+
589+
self.assertEqual(cursor.rowcount, -1)
590+
482591
def test_setinputsizes_a_noop(self):
483592
cursor = client.Cursor(Mock(), Mock())
484593
cursor.setinputsizes(1)

0 commit comments

Comments
 (0)