Skip to content

Commit 81bd16d

Browse files
committed
[SEA-NodeJS] Sync execute via directResults: fix CREATE, drop close-drives, keep mid-run cancel
The default sync path now calls the kernel's folded directResults `execute()` (`Connection.executeStatement`), which returns a SINGLE `AsyncStatement` handle. The session wraps it with the operation backend's existing `asyncStatement` arm — no arm to feature-detect. The returned handle always corresponds to a server-owned statement: - fire-and-forget `CREATE`/`INSERT` commits (the server runs it inline during the POST) — fixes the prior lazy-execute "CREATE didn't run" bug; - `close()` is a clean release, never a drive-to-terminal — the eager-handle + close-drives workaround is gone; - a long query is cancellable via `op.cancel()` (`asyncStatement.cancel()`) once the handle is held (~80-300ms), at parity with the Thrift backend; - errors surface at `executeStatement`, matching Thrift / Python use_kernel. On the fast path the kernel handle is seeded with the inline result, so the first fetch/status is served with zero extra round-trips (and is 404-proof — a terminal handle never polls a released statement). A warehouse that auto-closes the statement (`state=CLOSED`) after delivering its inline result is handled by the kernel mapping `CLOSED -> Succeeded`, so this path no longer throws `OperationStateError(Closed)` on those warehouses. Requires the kernel folded directResults execute (databricks/databricks-sql-kernel#136). Regenerated napi router types (`executeStatement` now returns `AsyncStatement`); fallback package names stay on the driver's canonical `@databricks/sql-kernel-*`. Validated e2e (pecotesting) against auto-closing AND non-auto-closing warehouses: CREATE fire-and-forget commits (12/12 = Thrift parity), fetch-then-close (12/12), mid-run cancel, close() cheap (~140ms) on an abandoned long query, SELECT 1 p50 179ms vs Thrift 168ms. Unit: SEA suite 247 passing; eslint clean. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
1 parent 4b9e16e commit 81bd16d

3 files changed

Lines changed: 53 additions & 38 deletions

File tree

lib/sea/SeaSessionBackend.ts

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -183,24 +183,31 @@ export default class SeaSessionBackend implements ISessionBackend {
183183
options.queryTimeout !== undefined ? numberToInt64(options.queryTimeout).toNumber() : undefined;
184184

185185
if (!runAsync) {
186-
// Sync path: forward `queryTimeoutSecs` to the napi options — the kernel
187-
// `execute()` honours it (server statement timeout).
186+
// DEFAULT — directResults (the Thrift / JDBC / Python use_sea model). The
187+
// kernel sends ExecuteStatement with the server inline wait (~10s default
188+
// + on_wait_timeout=CONTINUE) and returns WITHOUT polling past it, as a
189+
// single `AsyncStatement` handle:
190+
// - a fast query comes back seeded with the inline result, so the first
191+
// fetch/status is served with zero extra round-trips (and is
192+
// 404-proof — a terminal handle never polls a released statement);
193+
// - a slow query comes back as a poll/cancel handle.
194+
// Either way the handle is tied to a server-owned statement, so a long
195+
// query stays cancellable (`asyncStatement.cancel()`) and `close()` is a
196+
// clean release — no eager-handle / close-drives workaround. Fire-and-
197+
// forget DDL/DML commits because the server runs it inline during the POST.
188198
const execOptions = this.buildExecuteOptions(options, queryTimeoutSecs);
189-
let cancellableExecution;
199+
let asyncStatement;
190200
try {
191-
cancellableExecution =
201+
asyncStatement =
192202
execOptions === undefined
193-
? await this.connection.executeStatementCancellable(statement)
194-
: await this.connection.executeStatementCancellable(statement, execOptions);
203+
? await this.connection.executeStatement(statement)
204+
: await this.connection.executeStatement(statement, execOptions);
195205
} catch (err) {
196206
throw this.logAndMapError('executeStatement', err);
197207
}
198208
return new SeaOperationBackend({
199-
cancellableExecution: cancellableExecution!,
209+
asyncStatement,
200210
context: this.context,
201-
// The kernel honours `queryTimeoutSecs` on the sync `execute` path, so
202-
// it is forwarded via the napi options (see `buildExecuteOptions`); the
203-
// backend also keeps it as a deadline guard for parity with async.
204211
queryTimeoutSecs,
205212
});
206213
}

native/sea/index.d.ts

Lines changed: 22 additions & 18 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/unit/sea/execution.test.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -210,28 +210,32 @@ class FakeNativeConnection implements SeaConnection {
210210
// sync `runAsync: false` query path — the DEFAULT).
211211
public lastCancellableExecution?: FakeCancellableExecution;
212212

213-
// The bare blocking executeStatement path: the SEA backend's sync default
214-
// routes through executeStatementCancellable (below), but the binding still
215-
// exposes this for completeness.
216-
public async executeStatement(sql: string, options?: unknown): Promise<SeaStatement> {
213+
// Sync (`runAsync: false`) cancellable query path (retained for the
214+
// executeStatementCancellable binding; the default execute path is the
215+
// directResults `executeStatement` below).
216+
public async executeStatementCancellable(sql: string, options?: unknown): Promise<any> {
217217
if (this.throwOnExecute) {
218218
throw this.throwOnExecute;
219219
}
220220
this.lastSql = sql;
221221
this.lastOptions = options;
222-
return this.statementToReturn;
222+
this.lastCancellableExecution = new FakeCancellableExecution();
223+
return this.lastCancellableExecution;
223224
}
224225

225-
// Sync (`runAsync: false`, the DEFAULT) query path: records sql + options and
226-
// returns a pending CancellableExecution whose result() drives the execute.
227-
public async executeStatementCancellable(sql: string, options?: unknown): Promise<any> {
226+
// directResults (`runAsync: false`, the DEFAULT) query path: records sql +
227+
// options and returns a single `AsyncStatement` handle — the kernel `execute()`
228+
// sends the inline-wait POST and returns one handle (seeded with the inline
229+
// result on the fast path, a poll/cancel handle otherwise). `submitStatusValue`
230+
// configures the state it reports.
231+
public async executeStatement(sql: string, options?: unknown): Promise<any> {
228232
if (this.throwOnExecute) {
229233
throw this.throwOnExecute;
230234
}
231235
this.lastSql = sql;
232236
this.lastOptions = options;
233-
this.lastCancellableExecution = new FakeCancellableExecution();
234-
return this.lastCancellableExecution;
237+
this.lastAsyncStatement = new FakeAsyncStatement(this.submitStatusValue);
238+
return this.lastAsyncStatement;
235239
}
236240

237241
// Async-submit path: records sql + per-statement options (for forwarding

0 commit comments

Comments
 (0)