fix(kernel): pass cursor row limit to kernel - #922
Conversation
There was a problem hiding this comment.
Verdict: 1 Medium
Looks good overall — the row_limit forwarding is wired correctly and the _drain() rewrite preserves buffer/offset/row-index accounting. One medium concern: the tests silently dropped the row_limit=0 boundary case, which the removed connector code treated as a real zero-row limit, leaving a possible behavior change (0 → unlimited, matching SEA) uncovered.
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — forwarding row_limit to the kernel and removing the connector-side stream truncation is coherent: set_row_limit is applied before both the sync and async execute branches, the _drain rewrite accounts for _buffer_offset/_buffered_count/_next_row_index consistently, and unit + e2e tests are updated (including the new None case). One low-severity note: negative row_limit was previously normalized to "unlimited" and that behavior is now dropped and untested.
Forward cursor
row_limitto the kernel statement and remove the temporary connector-side stream truncation. This uses databricks-sql-kernel#269 and advancesKERNEL_REVto the merged commit.Validation:
python -m pytest tests/unit -m 'not realkernel'(895 passed, 4 skipped). Live kernel coverage exercisesNone,0,1, and5.PECOBLR-4127