Skip to content

Commit 63a8761

Browse files
committed
[SEA-NodeJS] Sync execute via directResults: fix CREATE, drop close-drives, keep mid-run cancel
The default sync path (`runAsync: false`) now calls the kernel's directResults execute (`Connection.executeStatementDirect`) instead of the cancellable-execute path. The kernel runs ExecuteStatement with a bounded server inline wait and returns WITHOUT polling past it: a fast query comes back as a terminal `Statement` (result inline), a slow one as an `AsyncStatement` (a poll/cancel handle). The session feature-detects the arm via `awaitResult` and wraps it with the operation backend's existing `statement` / `asyncStatement` arms. Because the returned handle always corresponds to a server-owned statement: - fire-and-forget `CREATE`/`INSERT` (execute then close, no fetch) COMMITS — the server runs it inline during the POST (fixes the prior lazy-execute "CREATE didn't run" bug); - `close()` is a clean release (DELETE), never a drive-to-terminal — the eager-handle + close-drives workaround is gone entirely; - a long query is cancellable via `op.cancel()` once the handle is held (~150-300ms), at parity with the Thrift backend; - errors surface at `executeStatement` (not silently at close), matching Thrift and Python use_kernel. Requires the kernel directResults execute (databricks/databricks-sql-kernel#136). The regenerated napi router types add `executeStatementDirect`; the fallback package names stay on the driver's canonical `@databricks/sql-kernel-*`. Validated e2e (pecotesting): CREATE fire-and-forget commits, dependent ordering, 100k read, error-at-execute, mid-run cancel, and close() cheap (~120ms) on an abandoned long query. Unit: SEA suite 247 passing; eslint clean. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
1 parent 4b9e16e commit 63a8761

3 files changed

Lines changed: 63 additions & 14 deletions

File tree

lib/sea/SeaSessionBackend.ts

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import InfoValue from '../dto/InfoValue';
3333
import HiveDriverError from '../errors/HiveDriverError';
3434
import ParameterError from '../errors/ParameterError';
3535
import { LogLevel } from '../contracts/IDBSQLLogger';
36-
import { SeaConnection, SeaNativeExecuteOptions, SeaStatement } from './SeaNativeLoader';
36+
import { SeaConnection, SeaNativeExecuteOptions, SeaStatement, SeaNativeAsyncStatement } from './SeaNativeLoader';
3737
import { decodeNapiKernelError } from './SeaErrorMapping';
3838
import { numberToInt64 } from '../thrift-backend/ThriftSessionBackend';
3939
import SeaOperationBackend from './SeaOperationBackend';
@@ -183,24 +183,37 @@ 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 model). The kernel sends
187+
// ExecuteStatement with a bounded server-side inline wait (10s +
188+
// on_wait_timeout=CONTINUE) and returns WITHOUT polling past it:
189+
// - a terminal `Statement` (results inline) for a fast query, or
190+
// - an `AsyncStatement` (a poll/cancel handle) for a slow one.
191+
// Either way the handle is tied to a server-owned statement, so a long
192+
// query stays cancellable and `close()` is a clean release — no
193+
// eager-handle / close-drives workaround. Fire-and-forget DDL/DML commits
194+
// because the server runs it inline during the POST (fast path).
188195
const execOptions = this.buildExecuteOptions(options, queryTimeoutSecs);
189-
let cancellableExecution;
196+
let direct;
190197
try {
191-
cancellableExecution =
198+
direct =
192199
execOptions === undefined
193-
? await this.connection.executeStatementCancellable(statement)
194-
: await this.connection.executeStatementCancellable(statement, execOptions);
200+
? await this.connection.executeStatementDirect(statement)
201+
: await this.connection.executeStatementDirect(statement, execOptions);
195202
} catch (err) {
196203
throw this.logAndMapError('executeStatement', err);
197204
}
205+
// Feature-detect the arm: only `AsyncStatement` exposes `awaitResult`.
206+
const asAsync = direct as unknown as SeaNativeAsyncStatement;
207+
if (direct !== null && typeof asAsync.awaitResult === 'function') {
208+
return new SeaOperationBackend({
209+
asyncStatement: asAsync,
210+
context: this.context,
211+
queryTimeoutSecs,
212+
});
213+
}
198214
return new SeaOperationBackend({
199-
cancellableExecution: cancellableExecution!,
215+
statement: direct as unknown as SeaStatement,
200216
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.
204217
queryTimeoutSecs,
205218
});
206219
}

native/sea/index.d.ts

Lines changed: 19 additions & 3 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: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,26 @@ class FakeNativeConnection implements SeaConnection {
234234
return this.lastCancellableExecution;
235235
}
236236

237+
// directResults (`runAsync: false`, the DEFAULT) query path: records sql +
238+
// options and returns either a terminal `Statement` (Completed arm) or — when
239+
// `directReturnsRunning` is set — a pending `AsyncStatement` (Running arm).
240+
// These are the two arms `SeaSessionBackend.executeStatement` feature-detects
241+
// via `awaitResult`.
242+
public directReturnsRunning = false;
243+
244+
public async executeStatementDirect(sql: string, options?: unknown): Promise<any> {
245+
if (this.throwOnExecute) {
246+
throw this.throwOnExecute;
247+
}
248+
this.lastSql = sql;
249+
this.lastOptions = options;
250+
if (this.directReturnsRunning) {
251+
this.lastAsyncStatement = new FakeAsyncStatement(this.submitStatusValue);
252+
return this.lastAsyncStatement;
253+
}
254+
return this.statementToReturn;
255+
}
256+
237257
// Async-submit path: records sql + per-statement options (for forwarding
238258
// assertions) and returns a pending AsyncStatement.
239259
public async submitStatement(sql: string, options?: unknown): Promise<any> {

0 commit comments

Comments
 (0)