Skip to content

fix(kernel): honor cursor row_limit - #916

Merged
vuanhphung merged 2 commits into
mainfrom
vu-phung/pecoblr-4127-kernel-row-limit
Aug 18, 2026
Merged

fix(kernel): honor cursor row_limit#916
vuanhphung merged 2 commits into
mainfrom
vu-phung/pecoblr-4127-kernel-row-limit

Conversation

@vuanhphung

Copy link
Copy Markdown
Collaborator

Fixes PECOBLR-4127.

The Kernel adapter now applies the cursor's row_limit at the streaming result boundary for row and Arrow fetches, including row_limit=0. Metadata result sets remain unaffected.

Tested with the focused Kernel unit suite (134 passed); added Kernel E2E coverage for limits 0, 1, and 5.


This PR was created with GitHub MCP.

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No issues identified by the review bot.

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: 1 Medium

Solid, well-tested fix — the buffer/limit mechanics (remaining-limit math, batch slicing, exact-boundary exhaustion, metadata paths left unlimited) are all correct and covered by unit + e2e tests. One medium concern: row_limit=0 is treated as a hard zero-row cap here, which diverges from the SEA backend where 0 means unlimited (requests.py:48) — a potential silent behavior change on backend switch worth reconciling.

Comment thread src/databricks/sql/backend/kernel/result_set.py
@vuanhphung
vuanhphung added this pull request to the merge queue Aug 18, 2026
Merged via the queue into main with commit 45bfee8 Aug 18, 2026
58 of 60 checks passed
@vuanhphung
vuanhphung deleted the vu-phung/pecoblr-4127-kernel-row-limit branch August 18, 2026 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants