Skip to content

Commit 45bfee8

Browse files
authored
fix(kernel): honor cursor row_limit (#916)
* fix(kernel): honor cursor row limit * test(kernel): cover exact row limit boundary
1 parent 8f4daee commit 45bfee8

5 files changed

Lines changed: 150 additions & 36 deletions

File tree

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,9 @@ def execute_command(
590590
# native exception) — wrap the construction so callers see a
591591
# mapped PEP 249 exception.
592592
try:
593-
return self._make_result_set(executed, cursor, command_id)
593+
return self._make_result_set(
594+
executed, cursor, command_id, row_limit=row_limit
595+
)
594596
except Exception as exc:
595597
raise _wrap_kernel_exception("execute_command", exc) from exc
596598

@@ -762,7 +764,9 @@ def get_execution_result(
762764
# ``KernelResultSet.__init__`` calls ``arrow_schema()`` which
763765
# can raise — map that to PEP 249 too.
764766
try:
765-
return self._make_result_set(stream, cursor, command_id)
767+
return self._make_result_set(
768+
stream, cursor, command_id, row_limit=cursor.row_limit
769+
)
766770
except Exception as exc:
767771
raise _wrap_kernel_exception("get_execution_result", exc) from exc
768772

@@ -773,6 +777,7 @@ def _make_result_set(
773777
kernel_handle: Any,
774778
cursor: "Cursor",
775779
command_id: CommandId,
780+
row_limit: Optional[int] = None,
776781
) -> "ResultSet":
777782
"""Build a ``KernelResultSet`` from any kernel handle. Used
778783
by sync execute, ``get_execution_result``, and all metadata
@@ -794,6 +799,7 @@ def _make_result_set(
794799
command_id=command_id,
795800
arraysize=cursor.arraysize,
796801
buffer_size_bytes=cursor.buffer_size_bytes,
802+
row_limit=row_limit,
797803
)
798804

799805
def _synthetic_command_id(self) -> CommandId:

src/databricks/sql/backend/kernel/result_set.py

Lines changed: 39 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
within a batch when ``n`` is smaller than the kernel's natural
2222
batch size; ``fetchall`` drains the whole stream.
2323
24+
When a cursor has ``row_limit`` set, this class caps the logical stream
25+
before rows reach any of the row or Arrow fetch APIs.
26+
2427
Note: ``buffer_size_bytes`` is accepted by the constructor for
2528
contract compatibility with the base ``ResultSet`` but is not
2629
consulted — the kernel backend currently caps buffering by rows
@@ -67,6 +70,7 @@ def __init__(
6770
command_id: CommandId,
6871
arraysize: int,
6972
buffer_size_bytes: int,
73+
row_limit: Optional[int] = None,
7074
):
7175
try:
7276
schema = kernel_handle.arrow_schema()
@@ -100,27 +104,55 @@ def __init__(
100104
# stays O(1) instead of walking the deque.
101105
self._buffered_count: int = 0
102106
self._exhausted: bool = False
107+
# The PyO3 kernel surface does not currently expose the core
108+
# StatementSpec row_limit setter. Enforce the cursor contract at
109+
# this streaming boundary until it does. Negative values retain the
110+
# existing unlimited behaviour; zero is a real zero-row limit.
111+
self._row_limit: Optional[int] = (
112+
row_limit if row_limit is not None and row_limit >= 0 else None
113+
)
114+
if self._row_limit == 0:
115+
self._mark_exhausted()
103116

104117
# ----- internal helpers -----
105118

119+
def _mark_exhausted(self) -> None:
120+
self._exhausted = True
121+
self.has_more_rows = False
122+
self.status = CommandState.SUCCEEDED
123+
124+
def _remaining_row_limit(self) -> Optional[int]:
125+
if self._row_limit is None:
126+
return None
127+
return max(
128+
0,
129+
self._row_limit - self._next_row_index - self._buffered_count,
130+
)
131+
106132
def _pull_one_batch(self) -> bool:
107133
"""Pull the next batch from the kernel into the local buffer.
108134
Returns True if a batch was added; False if the kernel side
109135
is exhausted."""
110136
if self._exhausted:
111137
return False
138+
remaining_limit = self._remaining_row_limit()
139+
if remaining_limit == 0:
140+
self._mark_exhausted()
141+
return False
112142
try:
113143
batch = self._kernel_handle.fetch_next_batch()
114144
except Exception as exc:
115145
raise wrap_kernel_exception("fetch_next_batch", exc) from exc
116146
if batch is None:
117-
self._exhausted = True
118-
self.has_more_rows = False
119-
self.status = CommandState.SUCCEEDED
147+
self._mark_exhausted()
120148
return False
149+
if remaining_limit is not None and batch.num_rows > remaining_limit:
150+
batch = batch.slice(0, remaining_limit)
121151
if batch.num_rows > 0:
122152
self._buffer.append(batch)
123153
self._buffered_count += batch.num_rows
154+
if remaining_limit is not None and batch.num_rows >= remaining_limit:
155+
self._mark_exhausted()
124156
return True
125157

126158
def _ensure_buffered(self, n_rows: int) -> int:
@@ -156,36 +188,10 @@ def _take_buffered(self, n: int) -> pyarrow.Table:
156188
return pyarrow.Table.from_batches(slices, schema=self._schema)
157189

158190
def _drain(self) -> pyarrow.Table:
159-
"""Consume everything left in the buffer + kernel stream
160-
and return as a single Table."""
161-
chunks: List[pyarrow.RecordBatch] = []
162-
if self._buffer and self._buffer_offset > 0:
163-
head = self._buffer.popleft()
164-
chunks.append(
165-
head.slice(self._buffer_offset, head.num_rows - self._buffer_offset)
166-
)
167-
self._buffer_offset = 0
168-
while self._buffer:
169-
chunks.append(self._buffer.popleft())
170-
if not self._exhausted:
171-
while True:
172-
try:
173-
batch = self._kernel_handle.fetch_next_batch()
174-
except Exception as exc:
175-
raise wrap_kernel_exception("fetch_next_batch", exc) from exc
176-
if batch is None:
177-
self._exhausted = True
178-
self.has_more_rows = False
179-
self.status = CommandState.SUCCEEDED
180-
break
181-
if batch.num_rows > 0:
182-
chunks.append(batch)
183-
rows = sum(c.num_rows for c in chunks)
184-
self._buffered_count = 0
185-
self._next_row_index += rows
186-
if not chunks:
187-
return pyarrow.Table.from_batches([], schema=self._schema)
188-
return pyarrow.Table.from_batches(chunks, schema=self._schema)
191+
"""Consume the remaining logical stream into one table."""
192+
while not self._exhausted:
193+
self._pull_one_batch()
194+
return self._take_buffered(self._buffered_count)
189195

190196
# ----- Arrow fetches -----
191197

tests/e2e/test_kernel_backend.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,13 @@ def test_fetchall_arrow(conn):
183183
assert table.column_names == ["a", "b"]
184184

185185

186+
@pytest.mark.parametrize("row_limit", [0, 1, 5])
187+
def test_cursor_row_limit(conn, row_limit):
188+
with conn.cursor(row_limit=row_limit) as cur:
189+
cur.execute("SELECT id FROM range(10) ORDER BY id")
190+
assert [row[0] for row in cur.fetchall()] == list(range(row_limit))
191+
192+
186193
# ─── Logging (Rust kernel -> Python logging bridge) ──────────────────────────
187194
#
188195
# Layer 3 of the logger-name drift guard (see also the Rust tests

tests/unit/test_kernel_client.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,38 @@ def test_execute_command_forwards_query_tags():
436436
assert stmt.execute.called
437437

438438

439+
def test_execute_command_applies_row_limit_to_result_set():
440+
c = _make_client()
441+
c._kernel_session = MagicMock()
442+
cursor = MagicMock()
443+
cursor.arraysize = 100
444+
cursor.buffer_size_bytes = 1024
445+
446+
stmt = MagicMock()
447+
stmt.execute.return_value = MagicMock(
448+
statement_id="stmt-id",
449+
arrow_schema=MagicMock(return_value=pa.schema([("x", pa.int64())])),
450+
)
451+
c._kernel_session.statement.return_value = stmt
452+
453+
result = c.execute_command(
454+
operation="SELECT * FROM range(10)",
455+
session_id=MagicMock(),
456+
max_rows=1,
457+
max_bytes=1,
458+
lz4_compression=False,
459+
cursor=cursor,
460+
use_cloud_fetch=False,
461+
parameters=[],
462+
async_op=False,
463+
enforce_embedded_schema_correctness=False,
464+
row_limit=5,
465+
)
466+
467+
assert result is not None
468+
assert result._row_limit == 5
469+
470+
439471
# ---------------------------------------------------------------------------
440472
# Staging / volume operations — fail loud (not silently no-op)
441473
# ---------------------------------------------------------------------------
@@ -777,11 +809,13 @@ def test_get_execution_result_attaches_by_id():
777809
cursor = MagicMock()
778810
cursor.arraysize = 100
779811
cursor.buffer_size_bytes = 1024
812+
cursor.row_limit = 5
780813
cid = CommandId.from_sea_statement_id("async-1")
781814

782815
rs = c.get_execution_result(cid, cursor=cursor)
783816

784817
assert rs is not None
818+
assert rs._row_limit == 5
785819
c._kernel_session.attach_async_statement.assert_called_with("async-1")
786820
handle.await_result.assert_called_once_with()
787821

@@ -1033,6 +1067,7 @@ def test_get_execution_result_is_re_callable():
10331067
cursor = MagicMock()
10341068
cursor.arraysize = 100
10351069
cursor.buffer_size_bytes = 1024
1070+
cursor.row_limit = None
10361071

10371072
rs1 = c.get_execution_result(cid, cursor=cursor)
10381073
rs2 = c.get_execution_result(cid, cursor=cursor)

tests/unit/test_kernel_result_set.py

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,15 @@ def __init__(self, schema: pa.Schema, batches):
2828
self._schema = schema
2929
self._batches: Deque[pa.RecordBatch] = deque(batches)
3030
self.closed = False
31+
self.fetch_calls = 0
3132

3233
def arrow_schema(self) -> pa.Schema:
3334
return self._schema
3435

3536
def fetch_next_batch(self):
3637
if self.closed:
3738
raise RuntimeError("fetched after close")
39+
self.fetch_calls += 1
3840
if not self._batches:
3941
return None
4042
return self._batches.popleft()
@@ -43,7 +45,7 @@ def close(self):
4345
self.closed = True
4446

4547

46-
def _make_rs(handle) -> KernelResultSet:
48+
def _make_rs(handle, row_limit=None) -> KernelResultSet:
4749
# The base ResultSet __init__ takes a `connection` ref it never
4850
# actually dereferences during these buffer tests, so a Mock is
4951
# fine.
@@ -56,6 +58,7 @@ def _make_rs(handle) -> KernelResultSet:
5658
command_id=CommandId.from_sea_statement_id("smoke-test"),
5759
arraysize=100,
5860
buffer_size_bytes=1024,
61+
row_limit=row_limit,
5962
)
6063

6164

@@ -140,6 +143,63 @@ def test_fetchall_rows(int_schema):
140143
assert [r[0] for r in rows] == [1, 2, 3]
141144

142145

146+
@pytest.mark.parametrize("row_limit", [0, 1, 5])
147+
def test_row_limit_caps_fetchall(int_schema, row_limit):
148+
handle = _FakeKernelHandle(
149+
int_schema,
150+
[_batch(int_schema, [0, 1, 2]), _batch(int_schema, list(range(3, 10)))],
151+
)
152+
rs = _make_rs(handle, row_limit=row_limit)
153+
154+
rows = rs.fetchall()
155+
156+
assert [row[0] for row in rows] == list(range(row_limit))
157+
assert rs.rownumber == row_limit
158+
159+
160+
def test_row_limit_applies_across_fetch_methods(int_schema):
161+
handle = _FakeKernelHandle(
162+
int_schema,
163+
[_batch(int_schema, [0, 1, 2]), _batch(int_schema, [3, 4, 5, 6])],
164+
)
165+
rs = _make_rs(handle, row_limit=5)
166+
167+
first = rs.fetchmany(2)
168+
third = rs.fetchone()
169+
rest = rs.fetchall_arrow()
170+
171+
assert [row[0] for row in first] == [0, 1]
172+
assert third is not None and third[0] == 2
173+
assert rest.column(0).to_pylist() == [3, 4]
174+
assert rs.fetchone() is None
175+
176+
177+
def test_row_limit_stops_before_fetching_extra_batches(int_schema):
178+
handle = _FakeKernelHandle(
179+
int_schema,
180+
[_batch(int_schema, [0, 1, 2]), _batch(int_schema, [3, 4, 5])],
181+
)
182+
rs = _make_rs(handle, row_limit=2)
183+
184+
assert rs.fetchall_arrow().column(0).to_pylist() == [0, 1]
185+
assert handle.fetch_calls == 1
186+
187+
188+
def test_row_limit_exact_batch_boundary_skips_exhaustion_fetch(int_schema):
189+
handle = _FakeKernelHandle(int_schema, [_batch(int_schema, [0, 1, 2])])
190+
rs = _make_rs(handle, row_limit=3)
191+
192+
assert rs.fetchall_arrow().column(0).to_pylist() == [0, 1, 2]
193+
assert handle.fetch_calls == 1
194+
195+
196+
def test_row_limit_larger_than_result_returns_all_rows(int_schema):
197+
handle = _FakeKernelHandle(int_schema, [_batch(int_schema, [1, 2, 3])])
198+
rs = _make_rs(handle, row_limit=10)
199+
200+
assert rs.fetchall_arrow().column(0).to_pylist() == [1, 2, 3]
201+
202+
143203
def test_fetchmany_negative_raises(int_schema):
144204
rs = _make_rs(_FakeKernelHandle(int_schema, []))
145205
with pytest.raises(ValueError):

0 commit comments

Comments
 (0)