From bb749d65f0e23329dff49aba9a2d46816295f6cf Mon Sep 17 00:00:00 2001 From: Madhavendra Rathore Date: Sat, 6 Jun 2026 16:05:15 +0000 Subject: [PATCH 1/6] [SEA-NodeJS] Sync execute via directResults (executeStatementDirect) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The default sync path (`runAsync: false`) now calls the kernel's additive `Connection.executeStatementDirect` instead of `executeStatementCancellable`. The kernel runs ExecuteStatement with a bounded server inline wait and returns WITHOUT polling past it; the session feature-detects the arm via `awaitResult`: a fast query comes back as a terminal `Statement` (result inline) → wrapped with the operation backend's `statement` arm; a slow one as an `AsyncStatement` → the `asyncStatement` arm. Because the returned handle always corresponds to a server-owned statement: - fire-and-forget CREATE/INSERT commits (server runs it inline in the POST); - `close()` is a clean release, never a drive-to-terminal (no close-drives); - a long query stays cancellable via `op.cancel()` (~150-300ms), Thrift parity; - errors surface at `executeStatement`, matching Thrift / Python use_kernel. Requires the kernel's additive directResults execute (databricks/databricks-sql-kernel#140). `execute()` on the kernel is unchanged, so Python `use_kernel` needs no change. Regenerated napi types add `executeStatementDirect`. Validated e2e (pecotesting): CREATE fire-and-forget commits, 100k read, error at execute, mid-run cancel, close() cheap (~120ms) on an abandoned long query. Unit: SEA suite 260 passing; eslint clean. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore --- lib/sea/SeaSessionBackend.ts | 32 +++++++++++++++++++++----------- native/sea/index.d.ts | 16 ++++++++++++++++ tests/unit/sea/execution.test.ts | 20 ++++++++++++++++++++ 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/lib/sea/SeaSessionBackend.ts b/lib/sea/SeaSessionBackend.ts index d593f87e..f205db60 100644 --- a/lib/sea/SeaSessionBackend.ts +++ b/lib/sea/SeaSessionBackend.ts @@ -33,7 +33,7 @@ import InfoValue from '../dto/InfoValue'; import HiveDriverError from '../errors/HiveDriverError'; import ParameterError from '../errors/ParameterError'; import { LogLevel } from '../contracts/IDBSQLLogger'; -import { SeaConnection, SeaNativeExecuteOptions, SeaStatement } from './SeaNativeLoader'; +import { SeaConnection, SeaNativeExecuteOptions, SeaStatement, SeaNativeAsyncStatement } from './SeaNativeLoader'; import { decodeNapiKernelError } from './SeaErrorMapping'; import { numberToInt64 } from '../thrift-backend/ThriftSessionBackend'; import SeaOperationBackend from './SeaOperationBackend'; @@ -183,24 +183,34 @@ export default class SeaSessionBackend implements ISessionBackend { options.queryTimeout !== undefined ? numberToInt64(options.queryTimeout).toNumber() : undefined; if (!runAsync) { - // Sync path: forward `queryTimeoutSecs` to the napi options — the kernel - // `execute()` honours it (server statement timeout). + // DEFAULT — directResults (the Thrift/JDBC model). The kernel's + // `executeStatementDirect` runs ExecuteStatement with a bounded server + // inline wait and returns WITHOUT polling past it: + // - a terminal `Statement` (result inline) for a fast query, or + // - an `AsyncStatement` (a poll/cancel handle) for a slow one. + // Either way the handle is tied to a server-owned statement, so a long + // query stays cancellable via `op.cancel()` and `close()` is a clean + // release (no client-side drive-to-terminal). Fire-and-forget DDL/DML + // commits because the server runs it inline during the POST. const execOptions = this.buildExecuteOptions(options, queryTimeoutSecs); - let cancellableExecution; + let direct; try { - cancellableExecution = + direct = execOptions === undefined - ? await this.connection.executeStatementCancellable(statement) - : await this.connection.executeStatementCancellable(statement, execOptions); + ? await this.connection.executeStatementDirect(statement) + : await this.connection.executeStatementDirect(statement, execOptions); } catch (err) { throw this.logAndMapError('executeStatement', err); } + // Feature-detect the arm: only `AsyncStatement` (the Running arm) exposes + // `awaitResult`; the terminal `Statement` (Completed arm) does not. + const asAsync = direct as unknown as SeaNativeAsyncStatement; + if (direct !== null && typeof asAsync.awaitResult === 'function') { + return new SeaOperationBackend({ asyncStatement: asAsync, context: this.context, queryTimeoutSecs }); + } return new SeaOperationBackend({ - cancellableExecution: cancellableExecution!, + statement: direct as unknown as SeaStatement, context: this.context, - // The kernel honours `queryTimeoutSecs` on the sync `execute` path, so - // it is forwarded via the napi options (see `buildExecuteOptions`); the - // backend also keeps it as a deadline guard for parity with async. queryTimeoutSecs, }); } diff --git a/native/sea/index.d.ts b/native/sea/index.d.ts index 7232a947..70f76100 100644 --- a/native/sea/index.d.ts +++ b/native/sea/index.d.ts @@ -716,6 +716,22 @@ export declare class Connection { * omission to keep the wire shape stable for the common case. */ executeStatement(sql: string, options?: ExecuteOptions | undefined | null): Promise + /** + * directResults execute — the Thrift/JDBC model. Sends ExecuteStatement + * with a bounded server-side inline wait (`wait_timeout=10s`, + * `on_wait_timeout=CONTINUE`) and returns WITHOUT polling past it: + * + * - a **`Statement`** (left arm) when the query finished within the inline + * wait — terminal, result ready inline, `close()` is a clean release; + * - an **`AsyncStatement`** (right arm) when it did not — a poll/cancel + * handle the caller drives (`status()` / `awaitResult()` / `cancel()`). + * + * JS distinguishes the arms by feature-detecting `awaitResult` (present + * only on `AsyncStatement`). This is the path that gives mid-run cancel for + * long queries WITHOUT the eager-handle / close-drives workaround: the + * returned handle always corresponds to a server-owned statement. + */ + executeStatementDirect(sql: string, options?: ExecuteOptions | undefined | null): Promise /** * Execute a SQL statement on the blocking (sync) path, but return a * `CancellableExecution` handle so a concurrent JS task can cancel diff --git a/tests/unit/sea/execution.test.ts b/tests/unit/sea/execution.test.ts index 81cdfadd..c3f2baea 100644 --- a/tests/unit/sea/execution.test.ts +++ b/tests/unit/sea/execution.test.ts @@ -234,6 +234,26 @@ class FakeNativeConnection implements SeaConnection { return this.lastCancellableExecution; } + // directResults (`runAsync: false`, the DEFAULT) query path: records sql + + // options and returns either a terminal `Statement` (Completed arm) or — when + // `directReturnsRunning` is set — a pending `AsyncStatement` (Running arm), + // the two arms `SeaSessionBackend.executeStatement` feature-detects via + // `awaitResult`. + public directReturnsRunning = false; + + public async executeStatementDirect(sql: string, options?: unknown): Promise { + if (this.throwOnExecute) { + throw this.throwOnExecute; + } + this.lastSql = sql; + this.lastOptions = options; + if (this.directReturnsRunning) { + this.lastAsyncStatement = new FakeAsyncStatement(this.submitStatusValue); + return this.lastAsyncStatement; + } + return this.statementToReturn; + } + // Async-submit path: records sql + per-statement options (for forwarding // assertions) and returns a pending AsyncStatement. public async submitStatement(sql: string, options?: unknown): Promise { From c1d66dc27ceff9b78d000244c2da7e2f40df81f3 Mon Sep 17 00:00:00 2001 From: Madhavendra Rathore Date: Sun, 7 Jun 2026 20:15:56 +0000 Subject: [PATCH 2/6] [SEA-NodeJS] Address review: test the AsyncStatement arm; type-guard; null + stale comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - F1: add coverage for the directResults Running (AsyncStatement) arm — the branch the PR exists to add. Three tests via the fake's `directReturnsRunning`: (a) a still-running query routes through the AsyncStatement arm and is driven via `status()`/`waitUntilReady()`; (b) `op.cancel()` reaches the running statement's `cancel()`; (c) contrast — a fast query routes through the terminal `Statement` arm and cancel reaches it there. Previously `directReturnsRunning` was never set, so the async arm had zero coverage. - F4: replace the `as unknown as` double-casts with a cast-free user-defined type guard `isSeaAsyncStatement` (the napi `Statement`/`AsyncStatement` are the exact alias types, so no laundering is needed). Idiomatic, narrows both arms. - F5: reject a null/undefined `direct` up front via `logAndMapError` (HiveDriverError) instead of the inconsistent `direct !== null` guard that let a null fall through to the `{statement}` arm and defer an opaque TypeError. - F3: update the stale `runAsync` selector comment (still described `executeStatementCancellable` + blocking `result()`) to directResults; fix the stale `executeStatementCancellable` comment in the test. - Document the `queryTimeout`→`wait_timeout=Ns`+CANCEL interaction (a timeout shorter than the query cancels it rather than returning the Running handle). SEA unit suite 263 passing (3 new); eslint clean. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore --- lib/sea/SeaSessionBackend.ts | 57 ++++++++++++++++++++++---------- tests/unit/sea/execution.test.ts | 41 ++++++++++++++++++++++- 2 files changed, 79 insertions(+), 19 deletions(-) diff --git a/lib/sea/SeaSessionBackend.ts b/lib/sea/SeaSessionBackend.ts index f205db60..f3998c90 100644 --- a/lib/sea/SeaSessionBackend.ts +++ b/lib/sea/SeaSessionBackend.ts @@ -67,6 +67,19 @@ export interface SeaSessionBackendOptions { * needed — that pattern was removed when the napi binding moved these * onto `openSession` to match pyo3. */ + +/** + * Narrow the directResults union (`executeStatementDirect`'s + * `Statement | AsyncStatement`) to the Running `AsyncStatement` arm. Only that + * arm exposes `awaitResult`; the terminal `Statement` (Completed arm) does not. + * Mirrors the kernel `DirectStatement::{Completed, Running}` discriminant, which + * the opaque napi classes can't carry on the wire — the `awaitResult` probe is + * the load-bearing feature-detect (see databricks-sql-kernel#140). + */ +function isSeaAsyncStatement(x: SeaStatement | SeaNativeAsyncStatement): x is SeaNativeAsyncStatement { + return typeof (x as SeaNativeAsyncStatement).awaitResult === 'function'; +} + export default class SeaSessionBackend implements ISessionBackend { private readonly connection: SeaConnection; @@ -164,13 +177,16 @@ export default class SeaSessionBackend implements ISessionBackend { // JSDoc in `IDBSQLSession` for the cross-backend contract. // // - DEFAULT (`runAsync` false/undefined) — SYNC. Route through - // `executeStatementCancellable`: the kernel blocks on `execute()` - // (server-side direct-results / poll-to-terminal), which is faster and, - // with the napi sync canceller, fully cancellable mid-COMPUTE. The - // blocking drive runs in the operation backend's `result()` (inside - // `waitUntilReady`, which the facade invokes lazily at first fetch). - // `queryTimeoutSecs` IS honoured on this path (forwarded to the napi - // options below) since the kernel `execute()` consults it. + // `executeStatementDirect` (the directResults model): the kernel sends + // ExecuteStatement with the server's inline wait and returns WITHOUT + // polling past it — a terminal `Statement` (result inline) for a fast + // query, or a still-running `AsyncStatement` (poll/cancel handle) for a + // slow one. The handle is server-owned, so a long query stays cancellable + // via `op.cancel()` and `close()` is a clean release (no client-side + // drive-to-terminal). `queryTimeoutSecs` IS honoured here (forwarded to + // the napi options below) — note the kernel sends it as `wait_timeout=Ns` + // + CANCEL, so a timeout shorter than the query cancels it (eager error) + // rather than returning the `Running` handle. // // - `runAsync: true` — ASYNC. Submit (`wait_timeout=0s`): the server // returns a pending `AsyncStatement` immediately while the query runs; @@ -193,7 +209,7 @@ export default class SeaSessionBackend implements ISessionBackend { // release (no client-side drive-to-terminal). Fire-and-forget DDL/DML // commits because the server runs it inline during the POST. const execOptions = this.buildExecuteOptions(options, queryTimeoutSecs); - let direct; + let direct: SeaStatement | SeaNativeAsyncStatement; try { direct = execOptions === undefined @@ -202,17 +218,22 @@ export default class SeaSessionBackend implements ISessionBackend { } catch (err) { throw this.logAndMapError('executeStatement', err); } - // Feature-detect the arm: only `AsyncStatement` (the Running arm) exposes - // `awaitResult`; the terminal `Statement` (Completed arm) does not. - const asAsync = direct as unknown as SeaNativeAsyncStatement; - if (direct !== null && typeof asAsync.awaitResult === 'function') { - return new SeaOperationBackend({ asyncStatement: asAsync, context: this.context, queryTimeoutSecs }); + // The kernel contract (`Promise`) never yields + // null/undefined; reject a non-handle up front so a contract violation + // surfaces as a mapped driver error here rather than an opaque `TypeError` + // deferred to first fetch/close. + if (direct === null || direct === undefined) { + throw this.logAndMapError( + 'executeStatement', + new HiveDriverError('SEA executeStatementDirect returned no statement handle'), + ); + } + // Feature-detect the arm via a type guard: only the Running `AsyncStatement` + // exposes `awaitResult`; the terminal `Statement` (Completed arm) does not. + if (isSeaAsyncStatement(direct)) { + return new SeaOperationBackend({ asyncStatement: direct, context: this.context, queryTimeoutSecs }); } - return new SeaOperationBackend({ - statement: direct as unknown as SeaStatement, - context: this.context, - queryTimeoutSecs, - }); + return new SeaOperationBackend({ statement: direct, context: this.context, queryTimeoutSecs }); } // Async path: do NOT forward `queryTimeoutSecs` (the kernel ignores it on diff --git a/tests/unit/sea/execution.test.ts b/tests/unit/sea/execution.test.ts index c3f2baea..7878f9f0 100644 --- a/tests/unit/sea/execution.test.ts +++ b/tests/unit/sea/execution.test.ts @@ -567,6 +567,45 @@ describe('SeaSessionBackend', () => { expect(op.id).to.be.a('string').and.have.length.greaterThan(0); }); + it('executeStatement (sync default) routes a still-running query through the AsyncStatement arm', async () => { + // The directResults Running arm — a query that did NOT finish within the + // server inline wait comes back as an AsyncStatement (poll/cancel handle). + // This is the branch the whole PR exists to add (Node mid-run cancel). + const connection = new FakeNativeConnection(); + connection.directReturnsRunning = true; + const session = makeSession(connection); + const op = await session.executeStatement('SELECT slow', {}); + expect(op).to.be.instanceOf(SeaOperationBackend); + // The Running arm was taken: an AsyncStatement was constructed + wired + // (not the terminal `statement` arm). + expect(connection.lastAsyncStatement, 'AsyncStatement (Running) arm should be taken').to.not.equal(undefined); + // Driving the op polls the async handle's status() — the polling arm. + await op.waitUntilReady(); + expect(connection.lastAsyncStatement!.statusCalls, 'async handle polled via status()').to.be.greaterThan(0); + }); + + it('executeStatement (sync default) AsyncStatement arm: op.cancel() reaches the running statement', async () => { + // The point of directResults on single-threaded Node: the returned op holds + // a handle to the still-running statement, so op.cancel() can abort it. + const connection = new FakeNativeConnection(); + connection.directReturnsRunning = true; + const session = makeSession(connection); + const op = await session.executeStatement('SELECT slow', {}); + await op.cancel(); + expect(connection.lastAsyncStatement!.cancelled, 'cancel reaches the running statement').to.equal(true); + }); + + it('executeStatement (sync default) routes a fast query through the terminal Statement arm', async () => { + // Contrast: a query that finished within the inline wait comes back as a + // terminal Statement (result inline) — no AsyncStatement is created. + const connection = new FakeNativeConnection(); // directReturnsRunning = false (default) + const session = makeSession(connection); + const op = await session.executeStatement('SELECT 1', {}); + expect(connection.lastAsyncStatement, 'no AsyncStatement arm for a terminal query').to.equal(undefined); + await op.cancel(); + expect(connection.statementToReturn.cancelled, 'cancel reaches the terminal statement').to.equal(true); + }); + it('executeStatement forwards ordinalParameters as napi positionalParams', async () => { const connection = new FakeNativeConnection(); const session = makeSession(connection); @@ -696,7 +735,7 @@ describe('SeaSessionBackend', () => { const envelope = `__databricks_error__:${JSON.stringify({ code: 'SqlError', message: 'SUBMIT_BOOM' })}`; for (const opts of [{}, { runAsync: true }]) { const connection = new FakeNativeConnection(); - connection.throwOnExecute = new Error(envelope); // fails executeStatementCancellable / submitStatement + connection.throwOnExecute = new Error(envelope); // fails executeStatementDirect / submitStatement const session = makeSession(connection); let thrown: unknown; try { From d29e6758df383bb6c94918f89f3b34b7ce5bd627 Mon Sep 17 00:00:00 2001 From: Madhavendra Rathore Date: Sun, 7 Jun 2026 20:16:46 +0000 Subject: [PATCH 3/6] [SEA-NodeJS] Regenerate napi .d.ts JSDoc to match corrected kernel direct-path docs executeStatementDirect JSDoc now reflects: no wait_timeout field (server default inline wait + auto-close), the awaitResult feature-detect contract, and the queryTimeout->CANCEL interaction. Doc-only; no API surface change. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore --- native/sea/index.d.ts | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/native/sea/index.d.ts b/native/sea/index.d.ts index 70f76100..fdfdf92c 100644 --- a/native/sea/index.d.ts +++ b/native/sea/index.d.ts @@ -718,8 +718,8 @@ export declare class Connection { executeStatement(sql: string, options?: ExecuteOptions | undefined | null): Promise /** * directResults execute — the Thrift/JDBC model. Sends ExecuteStatement - * with a bounded server-side inline wait (`wait_timeout=10s`, - * `on_wait_timeout=CONTINUE`) and returns WITHOUT polling past it: + * with no `wait_timeout` field (server applies its ~10s default inline wait + * and auto-closes on success) and returns WITHOUT polling past it: * * - a **`Statement`** (left arm) when the query finished within the inline * wait — terminal, result ready inline, `close()` is a clean release; @@ -730,6 +730,13 @@ export declare class Connection { * only on `AsyncStatement`). This is the path that gives mid-run cancel for * long queries WITHOUT the eager-handle / close-drives workaround: the * returned handle always corresponds to a server-owned statement. + * + * **Load-bearing contract:** the kernel's `DirectStatement::{Completed, + * Running}` discriminant cannot ride on these opaque `#[napi]` classes, so + * consumers MUST feature-detect via `awaitResult` (the only member unique to + * `AsyncStatement`). `Statement` (the Completed arm) MUST NOT gain an + * `awaitResult` member, or every consumer silently misroutes. The pyo3 + * binding makes the same `await_result`-probe assumption. */ executeStatementDirect(sql: string, options?: ExecuteOptions | undefined | null): Promise /** @@ -895,11 +902,10 @@ export declare class Statement { * data note:** may contain SQL fragments or parameter values — * redact before centralised logging. * - * Populated on `Succeeded` / `Closed-with-inline-data` paths. - * On terminal-error states (`Failed` / `Cancelled` / - * `Closed-no-data`) the kernel returns an Error instead of a - * `Statement`, and the same field rides on the JS Error envelope - * under the same `displayMessage` key. + * Populated on `Succeeded` / `Closed` paths (incl. an empty `Closed`). + * On terminal-error states (`Failed` / `Cancelled`) the kernel returns + * an Error instead of a `Statement`, and the same field rides on the JS + * Error envelope under the same `displayMessage` key. */ displayMessage(): Promise /** From 8f1b268e9db989eb382ad95ff8c0af5c4f66e8e8 Mon Sep 17 00:00:00 2001 From: Madhavendra Rathore Date: Sun, 7 Jun 2026 20:28:14 +0000 Subject: [PATCH 4/6] [SEA-NodeJS] Make queryTimeout a no-op on SEA (stop abusing wait_timeout) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `queryTimeout` was mapped to the kernel's `wait_timeout` (via `query_timeout_secs` → `wait_timeout={N}s` + CANCEL). That is semantically wrong: `wait_timeout` is the server's inline-HOLD window (how long the POST blocks for results, capped 5–50s), NOT a statement-execution timeout. Mapping queryTimeout there silently caps it at 50s, rejects <5s with HTTP 400, and conflates the inline wait with execution. Per the option's own JSDoc, `queryTimeout` is "effective only with Compute clusters; for SQL Warehouses use the STATEMENT_TIMEOUT configuration" — and SEA targets SQL Warehouses. So make it a clean no-op on the SEA backend: not forwarded to the kernel (sync directResults path no longer sends `query_timeout_secs`), and not applied as a client-side deadline (async path). Drop the now-dead `buildExecuteOptions` param and the unused `numberToInt64` import. Verified e2e: `queryTimeout: 5` on a ~35s query previously cancelled at ~5s; it now runs to completion (no-op). directResults CREATE/close/cancel unaffected. Tests: sync + async paths now assert queryTimeout is NOT forwarded; removed the obsolete Int64 client-side-deadline test. SEA suite 262 passing; eslint clean. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore --- lib/sea/SeaSessionBackend.ts | 65 ++++++++++++++------------------ tests/unit/sea/execution.test.ts | 36 +++--------------- 2 files changed, 34 insertions(+), 67 deletions(-) diff --git a/lib/sea/SeaSessionBackend.ts b/lib/sea/SeaSessionBackend.ts index f3998c90..34188ac6 100644 --- a/lib/sea/SeaSessionBackend.ts +++ b/lib/sea/SeaSessionBackend.ts @@ -35,7 +35,6 @@ import ParameterError from '../errors/ParameterError'; import { LogLevel } from '../contracts/IDBSQLLogger'; import { SeaConnection, SeaNativeExecuteOptions, SeaStatement, SeaNativeAsyncStatement } from './SeaNativeLoader'; import { decodeNapiKernelError } from './SeaErrorMapping'; -import { numberToInt64 } from '../thrift-backend/ThriftSessionBackend'; import SeaOperationBackend from './SeaOperationBackend'; import { buildSeaPositionalParams, buildSeaNamedParams } from './SeaPositionalParams'; import { seaServerInfoValue } from './SeaServerInfo'; @@ -135,9 +134,9 @@ export default class SeaSessionBackend implements ISessionBackend { * Per-statement options forwarded to the kernel `ExecuteOptions`: * - `ordinalParameters` / `namedParameters` → bound params (mutually * exclusive — the kernel binds one placeholder style per statement); - * - `queryTimeout` → enforced client-side by the operation backend's poll - * deadline (the kernel ignores `queryTimeoutSecs` on the async submit - * path), NOT forwarded to the napi options; + * - `queryTimeout` → NO-OP on SEA (SQL Warehouses use `STATEMENT_TIMEOUT`); + * never forwarded to the kernel and never applied as a client-side + * deadline — see the note in `executeStatement`; * - `rowLimit` → `rowLimit` (SEA-only server-side row cap); * - `queryTags` → serialised into the conf overlay's reserved * `query_tags` key (the same wire shape Thrift's `serializeQueryTags` @@ -183,20 +182,25 @@ export default class SeaSessionBackend implements ISessionBackend { // query, or a still-running `AsyncStatement` (poll/cancel handle) for a // slow one. The handle is server-owned, so a long query stays cancellable // via `op.cancel()` and `close()` is a clean release (no client-side - // drive-to-terminal). `queryTimeoutSecs` IS honoured here (forwarded to - // the napi options below) — note the kernel sends it as `wait_timeout=Ns` - // + CANCEL, so a timeout shorter than the query cancels it (eager error) - // rather than returning the `Running` handle. + // drive-to-terminal). `queryTimeout` is a no-op here (see the note + // below) — it is NOT mapped to the server `wait_timeout`. // // - `runAsync: true` — ASYNC. Submit (`wait_timeout=0s`): the server // returns a pending `AsyncStatement` immediately while the query runs; // the backend polls `status()` to terminal in `waitUntilReady()` and - // materialises results via `awaitResult()`. `queryTimeoutSecs` is - // ignored by the kernel on submit, so it is enforced client-side by the - // operation backend's poll-loop deadline instead. + // materialises results via `awaitResult()`. `queryTimeout` is a no-op + // here too. const runAsync = options.runAsync ?? false; - const queryTimeoutSecs = - options.queryTimeout !== undefined ? numberToInt64(options.queryTimeout).toNumber() : undefined; + // `queryTimeout` is a NO-OP on the SEA (kernel) backend. It is the JDBC + // `setQueryTimeout` knob which — per the option's JSDoc — is effective only + // on Compute clusters; SQL Warehouses (what SEA targets) use the + // `STATEMENT_TIMEOUT` SQL/session conf instead. We deliberately do NOT map it + // to the SEA `wait_timeout` field: `wait_timeout` is the server's inline-hold + // window (the time the POST blocks for results, capped 5–50s), NOT a + // statement-execution timeout — mapping it there silently caps the timeout at + // 50s, rejects <5s with HTTP 400, and conflates the inline wait with + // execution. So `queryTimeout` is neither forwarded to the kernel nor used as + // a client-side deadline. if (!runAsync) { // DEFAULT — directResults (the Thrift/JDBC model). The kernel's @@ -208,7 +212,7 @@ export default class SeaSessionBackend implements ISessionBackend { // query stays cancellable via `op.cancel()` and `close()` is a clean // release (no client-side drive-to-terminal). Fire-and-forget DDL/DML // commits because the server runs it inline during the POST. - const execOptions = this.buildExecuteOptions(options, queryTimeoutSecs); + const execOptions = this.buildExecuteOptions(options); let direct: SeaStatement | SeaNativeAsyncStatement; try { direct = @@ -231,13 +235,13 @@ export default class SeaSessionBackend implements ISessionBackend { // Feature-detect the arm via a type guard: only the Running `AsyncStatement` // exposes `awaitResult`; the terminal `Statement` (Completed arm) does not. if (isSeaAsyncStatement(direct)) { - return new SeaOperationBackend({ asyncStatement: direct, context: this.context, queryTimeoutSecs }); + return new SeaOperationBackend({ asyncStatement: direct, context: this.context }); } - return new SeaOperationBackend({ statement: direct, context: this.context, queryTimeoutSecs }); + return new SeaOperationBackend({ statement: direct, context: this.context }); } - // Async path: do NOT forward `queryTimeoutSecs` (the kernel ignores it on - // submit — `wait_timeout=0s`); it is enforced client-side by the poll loop. + // Async path: submit (`wait_timeout=0s`) returns a pending handle the + // backend polls. (`queryTimeout` is a no-op on SEA — see the note above.) const execOptions = this.buildExecuteOptions(options); let asyncStatement; try { @@ -248,16 +252,11 @@ export default class SeaSessionBackend implements ISessionBackend { } catch (err) { throw this.logAndMapError('executeStatement', err); } - // `queryTimeout` is enforced client-side by the operation backend's poll - // loop: the kernel ignores `queryTimeoutSecs` on the async submit path - // (`submitStatement` always sends `wait_timeout=0s`), so we do NOT forward - // it to the napi options — passing it there would be a silent no-op. + // `queryTimeout` is a no-op on SEA (see the note at the top of this method); + // not forwarded to the kernel and not applied as a client-side deadline. return new SeaOperationBackend({ asyncStatement: asyncStatement!, context: this.context, - // `queryTimeout` is typed `number | bigint | Int64`; `numberToInt64(...).toNumber()` - // coerces all three (a bare `Number(int64)` yields NaN — node-int64 has no valueOf). - queryTimeoutSecs, }); } @@ -266,10 +265,7 @@ export default class SeaSessionBackend implements ISessionBackend { * `ExecuteOptions`, returning `undefined` when nothing is set so the * no-options call shape (`executeStatement(sql)`) is preserved. */ - private buildExecuteOptions( - options: ExecuteStatementOptions, - queryTimeoutSecs?: number, - ): SeaNativeExecuteOptions | undefined { + private buildExecuteOptions(options: ExecuteStatementOptions): SeaNativeExecuteOptions | undefined { // Positional (`?`) and named (`:name`) parameters are mutually exclusive — // the kernel binds one placeholder style per statement. Use the SAME error // type and message as the Thrift backend (`ThriftSessionBackend`) so a @@ -287,14 +283,9 @@ export default class SeaSessionBackend implements ISessionBackend { if (namedParams !== undefined) { execOptions.namedParams = namedParams; } - // `queryTimeoutSecs` is forwarded only on the SYNC path (the caller passes - // it in): the kernel `execute()` consults it as the server statement - // timeout. On the async submit path the caller omits it (the kernel ignores - // it under `wait_timeout=0s`), so it is enforced client-side by the - // operation backend's poll-loop deadline instead (see executeStatement). - if (queryTimeoutSecs !== undefined) { - execOptions.queryTimeoutSecs = queryTimeoutSecs; - } + // NB: `queryTimeout` is intentionally NOT forwarded — it is a no-op on SEA + // (SQL Warehouses use `STATEMENT_TIMEOUT`; mapping it to `wait_timeout` would + // abuse the inline-hold window). See the note in `executeStatement`. if (options.rowLimit !== undefined) { execOptions.rowLimit = Number(options.rowLimit); } diff --git a/tests/unit/sea/execution.test.ts b/tests/unit/sea/execution.test.ts index 7878f9f0..e5be050e 100644 --- a/tests/unit/sea/execution.test.ts +++ b/tests/unit/sea/execution.test.ts @@ -14,7 +14,6 @@ import { expect } from 'chai'; import sinon from 'sinon'; -import Int64 from 'node-int64'; import SeaBackend from '../../../lib/sea/SeaBackend'; import SeaSessionBackend from '../../../lib/sea/SeaSessionBackend'; import SeaOperationBackend from '../../../lib/sea/SeaOperationBackend'; @@ -650,46 +649,23 @@ describe('SeaSessionBackend', () => { expect((thrown as Error).message).to.equal('Driver does not support both ordinal and named parameters.'); }); - it('executeStatement (sync default) DOES forward queryTimeout to the napi options', async () => { + it('executeStatement (sync default) does NOT forward queryTimeout — no-op on SEA', async () => { const connection = new FakeNativeConnection(); const session = makeSession(connection); await session.executeStatement('SELECT 1', { queryTimeout: 30 }); - // Sync path: the kernel `execute()` honours queryTimeoutSecs (server - // statement timeout), so the backend forwards it onto the napi options. - expect((connection.lastOptions as { queryTimeoutSecs?: number } | undefined)?.queryTimeoutSecs).to.equal(30); + // queryTimeout is a no-op on SEA (SQL Warehouses use STATEMENT_TIMEOUT). It + // must NOT be mapped to the kernel's `wait_timeout` (the inline-hold window), + // so nothing is forwarded onto the napi options. + expect((connection.lastOptions as { queryTimeoutSecs?: number } | undefined)?.queryTimeoutSecs).to.equal(undefined); }); - it('executeStatement (runAsync: true) does NOT forward queryTimeout to submit (kernel ignores it; enforced client-side)', async () => { + it('executeStatement (runAsync: true) does NOT forward queryTimeout — no-op on SEA', async () => { const connection = new FakeNativeConnection(); const session = makeSession(connection); await session.executeStatement('SELECT 1', { queryTimeout: 30, runAsync: true }); - // Async submit path: the kernel ignores queryTimeoutSecs under - // `wait_timeout=0s`, so it's enforced client-side by the poll deadline - // instead — never forwarded to the napi options. expect((connection.lastOptions as { queryTimeoutSecs?: number } | undefined)?.queryTimeoutSecs).to.equal(undefined); }); - it('coerces an Int64 queryTimeout into the client-side deadline on the async path (not NaN)', async function int64Timeout() { - // Regression: `Number(new Int64(...))` yields NaN (node-int64 has no valueOf), - // which would silently disable the deadline. The backend must coerce via - // numberToInt64(...).toNumber() so an Int64 queryTimeout still bounds the poll. - // Exercised on the async path, where the client-side poll deadline applies. - // eslint-disable-next-line no-invalid-this - this.timeout(5000); - const connection = new FakeNativeConnection(); - connection.submitStatusValue = 'Running'; // never reaches a terminal state - const session = makeSession(connection); - const op = await session.executeStatement('SELECT 1', { queryTimeout: new Int64(1), runAsync: true }); - let thrown: unknown; - try { - await op.waitUntilReady(); - } catch (err) { - thrown = err; - } - expect(thrown, 'Int64(1) timeout must fire — NaN would poll forever').to.be.instanceOf(OperationStateError); - expect((thrown as OperationStateError).errorCode).to.equal(OperationStateErrorCode.Timeout); - }); - it('executeStatement forwards rowLimit', async () => { const connection = new FakeNativeConnection(); const session = makeSession(connection); From 11f0576db30ac3501151afc5e975e91f158a6795 Mon Sep 17 00:00:00 2001 From: Madhavendra Rathore Date: Sun, 7 Jun 2026 21:16:05 +0000 Subject: [PATCH 5/6] [SEA-NodeJS] Remove the dead queryTimeout deadline plumbing (kernel field gone) Follow-on to making queryTimeout a no-op on SEA: now that the kernel's `query_timeout_secs` field is removed (databricks/databricks-sql-kernel#140) and SeaSessionBackend no longer passes a timeout, the SeaOperationBackend client-side deadline is dead code. Remove it: - `SeaOperationBackend`: drop the `queryTimeoutSecs` option, the `queryTimeoutMs` field, and the async poll-loop deadline enforcement (the `OperationStateError(Timeout)` + best-effort-cancel branch). - Regenerated napi `.d.ts`: `ExecuteOptions` no longer carries `queryTimeoutSecs`. - Tests: remove the obsolete client-side-deadline test; drop the `queryTimeoutSecs` args from the `makeAsyncOp` / `makeSyncOp` helpers. - Comment cleanups in SeaNativeLoader. queryTimeout remains a no-op on SEA (SQL Warehouses use STATEMENT_TIMEOUT). SEA unit suite 261 passing; eslint clean; e2e directResults (CREATE/close/cancel) unaffected. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore --- lib/sea/SeaNativeLoader.ts | 2 +- lib/sea/SeaOperationBackend.ts | 46 +++----------------------------- native/sea/index.d.ts | 39 ++++++--------------------- tests/unit/sea/execution.test.ts | 23 +++------------- 4 files changed, 15 insertions(+), 95 deletions(-) diff --git a/lib/sea/SeaNativeLoader.ts b/lib/sea/SeaNativeLoader.ts index 07fb3f99..e7be5e9b 100644 --- a/lib/sea/SeaNativeLoader.ts +++ b/lib/sea/SeaNativeLoader.ts @@ -56,7 +56,7 @@ export type SeaStatement = NativeStatement; // Per-statement execution options and bound-parameter inputs are kernel // concerns: the napi binding generates the canonical shapes (`positionalParams` // / `namedParams` as `TypedValueInput` / `NamedTypedValueInput`, plus -// `rowLimit`, `queryTimeoutSecs`, `statementConf`, `queryTags`). We re-export +// `rowLimit`, `statementConf`, `queryTags`). We re-export // rather than re-declare so the driver-side param codec can never drift from // the kernel contract. export type SeaNativeExecuteOptions = NativeExecuteOptions; diff --git a/lib/sea/SeaOperationBackend.ts b/lib/sea/SeaOperationBackend.ts index 2a4c8136..e5be686a 100644 --- a/lib/sea/SeaOperationBackend.ts +++ b/lib/sea/SeaOperationBackend.ts @@ -140,15 +140,6 @@ export interface SeaOperationBackendOptions { * handle exposes one, else a fresh UUIDv4. */ id?: string; - /** - * Client-side query timeout in whole seconds (the public `queryTimeout`). - * The kernel ignores `queryTimeoutSecs` on the async submit path - * (`submitStatement` always sends `wait_timeout=0s`), so the JS poll loop - * enforces it as a deadline — on expiry it best-effort cancels the statement - * and throws `OperationStateError(Timeout)`, matching the Thrift path's - * server-side TIMEDOUT outcome. Omitted ⇒ no client-side deadline. - */ - queryTimeoutSecs?: number; } export default class SeaOperationBackend implements IOperationBackend { @@ -190,17 +181,12 @@ export default class SeaOperationBackend implements IOperationBackend { // already-terminal statement. Drives both fetch and result-metadata. private fetchHandlePromise?: Promise; - // Client-side query-timeout deadline in ms (the public `queryTimeout`), - // undefined when unset. Enforced in the async poll loop. - private readonly queryTimeoutMs?: number; - constructor({ asyncStatement, statement, cancellableExecution, context, id, - queryTimeoutSecs, }: SeaOperationBackendOptions) { // Exactly one of the three handle kinds must be supplied. const providedCount = @@ -232,7 +218,6 @@ export default class SeaOperationBackend implements IOperationBackend { this.context = context; this._id = id ?? asyncStatement?.statementId ?? statement?.statementId ?? cancellableExecution?.statementId ?? uuidv4(); - this.queryTimeoutMs = queryTimeoutSecs !== undefined && queryTimeoutSecs > 0 ? queryTimeoutSecs * 1000 : undefined; } public get id(): string { @@ -441,9 +426,6 @@ export default class SeaOperationBackend implements IOperationBackend { if (this.fetchHandlePromise) { return; } - // Client-side timeout deadline: the kernel ignores queryTimeoutSecs on the - // async submit path, so we enforce the public `queryTimeout` here. - const deadline = this.queryTimeoutMs !== undefined ? Date.now() + this.queryTimeoutMs : undefined; for (;;) { // A JS-initiated cancel/close short-circuits before the next poll. failIfNotActive(this.lifecycle); @@ -494,30 +476,8 @@ export default class SeaOperationBackend implements IOperationBackend { throw new OperationStateError(OperationStateErrorCode.Unknown); } - // Still Pending/Running — enforce the client-side timeout before sleeping. - if (deadline !== undefined && Date.now() >= deadline) { - // Best-effort server-side cancel so the statement doesn't keep running - // after we stop waiting; never mask the timeout with a cancel failure, - // but warn-log a failed cancel so a still-running server statement is - // diagnosable (mirrors the fetch-error cleanup path). - // eslint-disable-next-line no-await-in-loop - await this.cancel().catch((cancelErr) => { - const cause = cancelErr instanceof Error ? cancelErr.message : String(cancelErr); - this.context - .getLogger() - .log( - LogLevel.warn, - `SEA query-timeout cleanup: cancel() failed for operation ${this._id}; the server-side ` + - `statement may keep running until the session is closed. Cause: ${cause}`, - ); - }); - // Release the statement handle too (cancel stops the work; close frees - // the handle) — best-effort, mirroring the other terminal branches. - // eslint-disable-next-line no-await-in-loop - await this.bestEffortClose(); - throw new OperationStateError(OperationStateErrorCode.Timeout); - } - + // Still Pending/Running — sleep before the next poll. (There is no + // client-side query-timeout deadline: `queryTimeout` is a no-op on SEA.) // eslint-disable-next-line no-await-in-loop await delay(STATUS_POLL_INTERVAL_MS); } @@ -526,7 +486,7 @@ export default class SeaOperationBackend implements IOperationBackend { /** * Sync (`runAsync: false`) execute path. Drives the blocking * `CancellableExecution.result()` to a terminal `Statement` (the kernel polls - * to completion server-side, honouring `queryTimeoutSecs` on this path). The + * to completion server-side). The * await is interruptible: a JS-initiated `cancel()` fires the detached * canceller, the server flips the statement terminal, and the parked * `result()` rejects with `Cancelled` — which we map to the typed diff --git a/native/sea/index.d.ts b/native/sea/index.d.ts index fdfdf92c..f8105ff6 100644 --- a/native/sea/index.d.ts +++ b/native/sea/index.d.ts @@ -17,10 +17,11 @@ * produces (`lib/utils/queryTags.ts`). Backslashes in keys are * doubled; backslash/colon/comma in values are backslash-escaped. * - * `rowLimit` (SEA `row_limit`) and `queryTimeoutSecs` (the per-statement - * server wait timeout) are exposed here and threaded onto the kernel + * `rowLimit` (SEA `row_limit`) is exposed here and threaded onto the kernel * `StatementSpec`. `positionalParams` (`?`) and `namedParams` (`:name`) * carry bound query parameters, decoded via `params::parse_typed_value`. + * (There is no `queryTimeoutSecs`: it abused the SEA `wait_timeout` inline-hold + * window and was removed — a real per-statement timeout is `STATEMENT_TIMEOUT`.) * * **Tag-order caveat (M4 parity note).** The napi `queryTags` field * is a Rust `HashMap` whose iteration order is @@ -66,25 +67,6 @@ export interface ExecuteOptions { * `StatementSpec.row_limit`. Omitted ⇒ no driver-imposed cap. */ rowLimit?: number - /** - * Per-statement server wait timeout in whole seconds. Bounds how - * long the server waits before cancelling the statement - * (`on_wait_timeout = CANCEL`), surfacing as a timeout — the - * server statement timeout (JDBC `setQueryTimeout`). Maps to - * `StatementSpec.query_timeout_secs`. Distinct from the - * connection-level transport timeout. The SEA wire caps it at 50s. - * - * **Applies to `executeStatement` only.** The async `submitStatement` - * path always sends `wait_timeout=0s` (so the server returns a - * `statement_id` immediately and never blocks), which means this - * field is *not* consulted on the submit path — the caller drives - * completion via `AsyncStatement.status()` / `awaitResult()` and is - * responsible for its own timeout (e.g. `Promise.race`). Passing - * `queryTimeoutSecs` to `submitStatement` is silently ignored. This - * matches the Python `use_sea` async-submit contract, where the - * wait-timeout and any statement timeout are orthogonal. - */ - queryTimeoutSecs?: number /** * Positional parameters, in 1-based wire order. Index `i` in this * Vec corresponds to the `i+1`-th `?` placeholder in the SQL. @@ -755,9 +737,7 @@ export declare class Connection { * `Statement` `executeStatement` returns) and can fire `cancel()` * concurrently to interrupt a still-running query mid-COMPUTE. * - * Option semantics are identical to `executeStatement` — including - * `queryTimeoutSecs`, which IS honoured here (this is the sync - * `execute` path; only the async `submitStatement` ignores it). + * Option semantics are identical to `executeStatement`. * Mirrors the pyo3 `Statement.canceller()` / `Statement.execute()` * split (PR #121): obtain the canceller before the blocking drive. */ @@ -775,14 +755,11 @@ export declare class Connection { * uses (`runAsync: true`): the SEA backend submits, returns a * pending operation handle, and polls to terminal during * fetch. Option semantics (statementConf / queryTags / - * rowLimit / positional + named params) match `executeStatement`, - * with one carve-out: `queryTimeoutSecs` is **ignored** here. + * rowLimit / positional + named params) match `executeStatement`. * Submit always sends `wait_timeout=0s` so the call returns - * immediately, leaving no server wait to bound; the caller drives - * completion and its own timeout via `status()` / `awaitResult()` - * (e.g. `Promise.race`). Only the blocking-vs-pending return - * contract — and that timeout carve-out — differ from - * `executeStatement`. + * immediately; the caller drives completion via `status()` / + * `awaitResult()`. Only the blocking-vs-pending return contract + * differs from `executeStatement`. */ submitStatement(sql: string, options?: ExecuteOptions | undefined | null): Promise /** diff --git a/tests/unit/sea/execution.test.ts b/tests/unit/sea/execution.test.ts index e5be050e..88b47e4d 100644 --- a/tests/unit/sea/execution.test.ts +++ b/tests/unit/sea/execution.test.ts @@ -909,9 +909,9 @@ describe('SeaOperationBackend', () => { }); describe('SeaOperationBackend — async (submitStatement) path', () => { - const makeAsyncOp = (asyncStatement: FakeAsyncStatement, queryTimeoutSecs?: number) => + const makeAsyncOp = (asyncStatement: FakeAsyncStatement) => // eslint-disable-next-line @typescript-eslint/no-explicit-any - new SeaOperationBackend({ asyncStatement: asyncStatement as any, context: makeContext(), queryTimeoutSecs }); + new SeaOperationBackend({ asyncStatement: asyncStatement as any, context: makeContext() }); it('rejects when neither asyncStatement nor statement is provided', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -1011,22 +1011,6 @@ describe('SeaOperationBackend — async (submitStatement) path', () => { expect((thrown as OperationStateError).errorCode).to.equal(OperationStateErrorCode.Closed); }); - it('waitUntilReady() enforces queryTimeout client-side: throws Timeout and cancels a stuck Running statement', async function timeoutTest() { - // eslint-disable-next-line no-invalid-this - this.timeout(5000); - const stmt = new FakeAsyncStatement('Running'); // never reaches a terminal state - const op = makeAsyncOp(stmt, 0.05); // 50ms client-side deadline - let thrown: unknown; - try { - await op.waitUntilReady(); - } catch (err) { - thrown = err; - } - expect(thrown).to.be.instanceOf(OperationStateError); - expect((thrown as OperationStateError).errorCode).to.equal(OperationStateErrorCode.Timeout); - // Best-effort server-side cancel fired so the statement doesn't keep running. - expect(stmt.cancelled).to.equal(true); - }); it('cancel() forwards to the async statement and short-circuits a subsequent poll', async () => { const stmt = new FakeAsyncStatement(['Running', 'Running', 'Succeeded']); @@ -1052,12 +1036,11 @@ describe('SeaOperationBackend — async (submitStatement) path', () => { }); describe('SeaOperationBackend — sync (executeStatementCancellable) path', () => { - const makeSyncOp = (cancellableExecution: FakeCancellableExecution, queryTimeoutSecs?: number) => + const makeSyncOp = (cancellableExecution: FakeCancellableExecution) => new SeaOperationBackend({ // eslint-disable-next-line @typescript-eslint/no-explicit-any cancellableExecution: cancellableExecution as any, context: makeContext(), - queryTimeoutSecs, }); it('rejects when more than one handle kind is provided', () => { From 82e549b41d06fb414beb1c57deae2e3a150c0534 Mon Sep 17 00:00:00 2001 From: Madhavendra Rathore Date: Mon, 8 Jun 2026 09:21:47 +0000 Subject: [PATCH 6/6] style: prettier-format SeaOperationBackend + execution.test after queryTimeout removal CI runs `prettier . --check` (not just eslint); the deadline-plumbing removal left two files needing a prettier reflow. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore --- lib/sea/SeaOperationBackend.ts | 8 +------- tests/unit/sea/execution.test.ts | 1 - 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/lib/sea/SeaOperationBackend.ts b/lib/sea/SeaOperationBackend.ts index e5be686a..7678b0a3 100644 --- a/lib/sea/SeaOperationBackend.ts +++ b/lib/sea/SeaOperationBackend.ts @@ -181,13 +181,7 @@ export default class SeaOperationBackend implements IOperationBackend { // already-terminal statement. Drives both fetch and result-metadata. private fetchHandlePromise?: Promise; - constructor({ - asyncStatement, - statement, - cancellableExecution, - context, - id, - }: SeaOperationBackendOptions) { + constructor({ asyncStatement, statement, cancellableExecution, context, id }: SeaOperationBackendOptions) { // Exactly one of the three handle kinds must be supplied. const providedCount = (asyncStatement !== undefined ? 1 : 0) + diff --git a/tests/unit/sea/execution.test.ts b/tests/unit/sea/execution.test.ts index 88b47e4d..0badd491 100644 --- a/tests/unit/sea/execution.test.ts +++ b/tests/unit/sea/execution.test.ts @@ -1011,7 +1011,6 @@ describe('SeaOperationBackend — async (submitStatement) path', () => { expect((thrown as OperationStateError).errorCode).to.equal(OperationStateErrorCode.Closed); }); - it('cancel() forwards to the async statement and short-circuits a subsequent poll', async () => { const stmt = new FakeAsyncStatement(['Running', 'Running', 'Succeeded']); const op = makeAsyncOp(stmt);