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..7678b0a3 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,18 +181,7 @@ 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) { + constructor({ asyncStatement, statement, cancellableExecution, context, id }: SeaOperationBackendOptions) { // Exactly one of the three handle kinds must be supplied. const providedCount = (asyncStatement !== undefined ? 1 : 0) + @@ -232,7 +212,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 +420,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 +470,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 +480,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/lib/sea/SeaSessionBackend.ts b/lib/sea/SeaSessionBackend.ts index d593f87e..34188ac6 100644 --- a/lib/sea/SeaSessionBackend.ts +++ b/lib/sea/SeaSessionBackend.ts @@ -33,9 +33,8 @@ 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'; import { buildSeaPositionalParams, buildSeaNamedParams } from './SeaPositionalParams'; import { seaServerInfoValue } from './SeaServerInfo'; @@ -67,6 +66,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; @@ -122,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` @@ -164,49 +176,72 @@ 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). `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) { - // Sync path: forward `queryTimeoutSecs` to the napi options — the kernel - // `execute()` honours it (server statement timeout). - const execOptions = this.buildExecuteOptions(options, queryTimeoutSecs); - let cancellableExecution; + // 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); + let direct: SeaStatement | SeaNativeAsyncStatement; 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); } - return new SeaOperationBackend({ - cancellableExecution: cancellableExecution!, - 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, - }); + // 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 }); + } + 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 { @@ -217,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, }); } @@ -235,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 @@ -256,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/native/sea/index.d.ts b/native/sea/index.d.ts index 7232a947..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. @@ -716,6 +698,29 @@ 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 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; + * - 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. + * + * **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 /** * Execute a SQL statement on the blocking (sync) path, but return a * `CancellableExecution` handle so a concurrent JS task can cancel @@ -732,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. */ @@ -752,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 /** @@ -879,11 +879,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 /** diff --git a/tests/unit/sea/execution.test.ts b/tests/unit/sea/execution.test.ts index 81cdfadd..0badd491 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'; @@ -234,6 +233,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 { @@ -547,6 +566,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); @@ -591,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); @@ -676,7 +711,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 { @@ -874,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 @@ -976,23 +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']); const op = makeAsyncOp(stmt); @@ -1017,12 +1035,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', () => {