| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
…V to batch 2 Connector half of the kernel batch-2 fixes (kernel PR #123). Bumps KERNEL_REV to pick up the batch-2 kernel surface. H4 — don't close the kernel Statement at sync execute-return: execute_command's finally used to `stmt.close()` immediately after `stmt.execute()` succeeded. For a large CloudFetch result with paginated chunk links (all_fetched=false), the kernel fetches later links lazily (get_result_chunks against the LIVE statement) during consumption, so a premature CloseStatement broke those fetches. The kernel now auto-closes the server statement when its result stream drains (ExecutedStatement::next_batch end-of-stream), with the executed-handle Drop as the backstop for partial/abandoned reads. So the connector now flips close_stmt=False on a successful execute and only closes on the error path (no executed handle was produced). The other batch-2 fixes (cancelled-class -> OperationalError, U2M refresh fail-fast, metadata statement close-on-drop, per-binding OAuth client_id) are entirely kernel-side and need no connector code beyond the KERNEL_REV bump. Tests: unit (sync execute does-not-close on success / does-close on failure) + e2e (large multi-chunk result drains without premature close + cursor reuse; server cancel maps to OperationalError not ProgrammingError). All e2e verified live against dogfood. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
#123 picked up a follow-up commit fixing the wiremock cancelled-state assertions (ErrorCode::Cancelled). Bump the placeholder pin so the connector CI builds the corrected kernel. Still to be re-pinned to the squash-merge SHA before #830 merges. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Kernel batch-2 (#123) is merged to kernel main (f4ee6fe, current main HEAD). Re-pin from the orphaned branch HEAD (4f7fbe7) to the merged SHA — reachable from main, no orphan-SHA risk. Verified against a wheel built from f4ee6fe: connector unit (102) + kernel e2e (H4 large-result drain + reuse, server-cancel -> OperationalError, staging fail-loud, diagnostic-info) all pass against the real merged kernel. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…atch2-fixes Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Address review: 'H4' is internal audit shorthand, meaningless in the public connector codebase. Reword the affected comment + two test docstrings to describe the behavior directly (premature sync CloseStatement broke lazy CloudFetch chunk-link fetches). No code change. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
| Back | FazBrowse Home | New Git URL |
What
Connector half of the kernel batch-2 CUJ-gap fixes (databricks-sql-kernel#123, merged). Pins KERNEL_REV to merged kernel main and adds the one connector-side change.
KERNEL_REV bump
KERNEL_REV → f4ee6fec78aabce8c0ea9c1ff47fc11b8191d013 — current kernel main, which includes the batch-2 kernel work (#123: sync-statement auto-close-on-drain, server-CANCELLED → cancelled class, U2M refresh fail-fast, metadata statement close-on-drop, per-binding OAuth client_id) plus #120 (logging bridge) and #125 (version bump). This is a merged, non-orphan SHA reachable from main.
Connector change — don't close the kernel Statement at sync execute-return
execute_command's finally used to stmt.close() immediately after a successful stmt.execute(). For a large CloudFetch result with paginated chunk links (all_fetched=false), the kernel fetches later links lazily (get_result_chunks against the live statement) during consumption — so a premature CloseStatement broke those fetches. (The common all_fetched=true case was unaffected, so this only bit very large paginated-link results.)
The kernel now auto-closes the server statement when its result stream drains (#123), with the executed-handle Drop as the backstop for partial/abandoned reads. So the connector now flips close_stmt = False on a successful execute and closes only on the error path (where no executed handle / result set was produced to reap the statement).
The other batch-2 fixes (cancelled-class → OperationalError, U2M refresh fail-fast, metadata close-on-drop, per-binding OAuth client_id) are entirely kernel-side and need no connector code beyond the KERNEL_REV bump.
Testing
Kernel #123 is merged; this is ready to merge.
This pull request and its description were written by Isaac.