From 75672e04ea0132d2589f01630325ed9367ec7758 Mon Sep 17 00:00:00 2001 From: Aakash Saravanan Date: Fri, 13 Nov 2026 23:01:51 +0000 Subject: [PATCH 1/8] Complete configured Volume operations in the native backend Signed-off-by: Aakash Saravanan --- CHANGELOG.md | 2 + CONNECTION_PARAMETERS.md | 8 +++- lib/DBSQLSession.ts | 15 +++++-- lib/contracts/IOperationBackend.ts | 2 + lib/kernel/KernelErrorMapping.ts | 24 ++++++++++- lib/kernel/KernelOperationBackend.ts | 36 ++++++++++++++-- lib/kernel/KernelSessionBackend.ts | 14 +------ native/kernel/index.d.ts | 4 ++ tests/unit/DBSQLSession.test.ts | 16 +++++++ .../kernel/KernelOperationBackend.test.ts | 42 +++++++++++++++++++ tests/unit/kernel/error-mapping.test.ts | 17 ++++++++ tests/unit/kernel/execution.test.ts | 10 +++-- 12 files changed, 167 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5eff56e6..f8d90e1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ## Unreleased - Reduce result chunking and `fetchAll()` overhead for large query results. +- Support Volume file transfers on the kernel backend when `stagingAllowedLocalPath` is supplied. Symlink targets must remain within configured allowed roots; in-root symlinks are permitted. + - Thrift CloudFetch downloads are now bounded end to end by `retriesTimeout` (default 15 minutes), including every attempt and retry backoff. Previously a stalled download, where the response headers or body never arrived, blocked the fetch forever. A download still running when the deadline expires is aborted and fails with `RetryError` (`TIMEOUT_EXCEEDED`, message `CloudFetch result download failed: Retry timeout exceeded`). (#520) - Thrift connections made through `proxy` now apply `checkServerCertificate`, `customCaCert` and `clientCert`/`clientKey`. Previously the proxied path ignored them and verified against Node's process-wide trust store, so a certificate outside the configured trust set could be accepted. **Compatibility:** proxied connections now verify unless `checkServerCertificate: false` is set; as on direct connections, `NODE_TLS_REJECT_UNAUTHORIZED=0` no longer turns verification off. For an `https` proxy these options also apply to the proxy's own certificate. (#485) - Honor the client `useCloudFetch: false` default on the kernel backend so complete results can be fetched without cloud-storage downloads. (#574) diff --git a/CONNECTION_PARAMETERS.md b/CONNECTION_PARAMETERS.md index e0503021..24ee78de 100644 --- a/CONNECTION_PARAMETERS.md +++ b/CONNECTION_PARAMETERS.md @@ -197,7 +197,7 @@ telemetry implementation, so only the options explicitly forwarded by | `maxRows` | `number \| bigint \| Int64 \| null` | ✅ | ❌ | `100000` / — | Thrift uses it for the initial Direct Results row count; `null` disables Direct Results. Kernel does not retain or forward the supplied value. | | `useCloudFetch` | `boolean` | ✅ | ❌ | `true` / — | Kernel logs and ignores this per-statement hint; Kernel result fetching owns its CloudFetch behavior. | | `useLZ4Compression` | `boolean` | ✅ | ❌ | `true` / — | Thrift uses it when supported and when the result is not CloudFetch. Kernel owns and auto-detects result compression. | -| `stagingAllowedLocalPath` | `string \| string[]` | ✅ | ❌ | — | Local allowlist for Thrift volume/staging operations. Kernel volume operations are unsupported. | +| `stagingAllowedLocalPath` | `string \| string[]` | ✅ | ✅ | — | Local allowlist for Volume operations. Kernel also requires resolved symlink targets within allowed roots; links within those roots are permitted. | | `namedParameters` | `Record` | ✅ | ✅ | — | Named SQL parameters. Cannot be combined with non-empty `ordinalParameters`. Thrift requires a protocol that supports parameterized queries. | | `ordinalParameters` | `Array` | ✅ | ✅ | — | Positional SQL parameters. Cannot be combined with non-empty `namedParameters`. | | `queryTags` | `Record` | ✅ | ✅ | — | Serialized into the per-statement `query_tags` conf overlay on both. | @@ -236,3 +236,9 @@ independent TLS hostname verification, optional U2M disk caching, and per-statement `rowLimit`/`statementConf`. The Kernel path remains internal and unstable; its behavior may change. + +With the kernel backend, supplying `stagingAllowedLocalPath` completes Volume +transfers before `executeStatement` resolves and consumes instruction rows. +Omitting the option returns raw instructions without file effects. Unlike the +Thrift backend’s lexical path check, the kernel also rejects symlink targets +outside the configured roots while permitting symlinks within them. diff --git a/lib/DBSQLSession.ts b/lib/DBSQLSession.ts index 5bca61d6..7633990f 100644 --- a/lib/DBSQLSession.ts +++ b/lib/DBSQLSession.ts @@ -109,11 +109,20 @@ export default class DBSQLSession implements IDBSQLSession { // operation, everything will work as usual. In a case of staging operation, it will be processed like any // other query - it will be possible to get data from it as usual, or use other operation methods. if (options.stagingAllowedLocalPath !== undefined) { + const allowedLocalPath = Array.isArray(options.stagingAllowedLocalPath) + ? options.stagingAllowedLocalPath + : [options.stagingAllowedLocalPath]; + if (opBackend.completeStagingOperation) { + try { + await this.runBackend(() => opBackend.completeStagingOperation!(allowedLocalPath)); + return operation; + } catch (error) { + await operation.close().catch(() => {}); + throw error; + } + } const metadata = await operation.getResultMetadata(); if (metadata.isStagingOperation) { - const allowedLocalPath = Array.isArray(options.stagingAllowedLocalPath) - ? options.stagingAllowedLocalPath - : [options.stagingAllowedLocalPath]; return this.handleStagingOperation(operation, allowedLocalPath, metadata); } } diff --git a/lib/contracts/IOperationBackend.ts b/lib/contracts/IOperationBackend.ts index 109de87e..791008d3 100644 --- a/lib/contracts/IOperationBackend.ts +++ b/lib/contracts/IOperationBackend.ts @@ -113,6 +113,8 @@ export default interface IOperationBackend { */ getCachedResultMetadata?(): ResultMetadata | undefined; + completeStagingOperation?(allowedPaths: string[]): Promise; + /** Cancel the operation. */ cancel(): Promise; diff --git a/lib/kernel/KernelErrorMapping.ts b/lib/kernel/KernelErrorMapping.ts index d83d7e59..fcd2f35b 100644 --- a/lib/kernel/KernelErrorMapping.ts +++ b/lib/kernel/KernelErrorMapping.ts @@ -1,3 +1,5 @@ +import { constants } from 'node:os'; +import { getSystemErrorName } from 'node:util'; import HiveDriverError from '../errors/HiveDriverError'; import AuthenticationError from '../errors/AuthenticationError'; import OperationStateError, { OperationStateErrorCode } from '../errors/OperationStateError'; @@ -311,7 +313,27 @@ export function decodeNapiKernelError(err: unknown): Error { const msg = envelope.message as string; const sqlState = typeof envelope.sqlState === 'string' ? envelope.sqlState : undefined; - const jsErr = mapKernelErrorToJsError({ code, message: msg, sqlstate: sqlState }); + let jsErr = mapKernelErrorToJsError({ code, message: msg, sqlstate: sqlState }); + if (typeof envelope.localPath === 'string' && typeof envelope.localErrorKind === 'string') { + const kindCodes: Record = { + NotFound: 'ENOENT', + PermissionDenied: 'EACCES', + AlreadyExists: 'EEXIST', + NotADirectory: 'ENOTDIR', + IsADirectory: 'EISDIR', + DirectoryNotEmpty: 'ENOTEMPTY', + }; + const raw = typeof envelope.localOsError === 'number' ? envelope.localOsError : undefined; + const localCode = + process.platform !== 'win32' && raw + ? getSystemErrorName(-Math.abs(raw)) + : kindCodes[envelope.localErrorKind] ?? 'EIO'; + jsErr = Object.assign(new Error(msg), { + code: localCode, + errno: -((constants.errno as Record)[localCode] ?? Math.abs(raw ?? constants.errno.EIO)), + path: envelope.localPath, + }); + } const meta = buildKernelMetadata(envelope); // Skip the namespace attachment entirely when no fields validated diff --git a/lib/kernel/KernelOperationBackend.ts b/lib/kernel/KernelOperationBackend.ts index 41f15de2..d74f7f60 100644 --- a/lib/kernel/KernelOperationBackend.ts +++ b/lib/kernel/KernelOperationBackend.ts @@ -46,6 +46,8 @@ import { LogLevel } from '../contracts/IDBSQLLogger'; import { RowMode } from '../contracts/IDBSQLSession'; import Status from '../dto/Status'; import HiveDriverError from '../errors/HiveDriverError'; +import StagingError from '../errors/StagingError'; +import ParameterError from '../errors/ParameterError'; import OperationStateError, { OperationStateErrorCode } from '../errors/OperationStateError'; import ArrowResultConverter from '../result/ArrowResultConverter'; import ResultSlicer from '../result/ResultSlicer'; @@ -84,7 +86,10 @@ export type KernelOperationStatement = KernelStatementHandle & Partial; +type KernelFetchHandle = Pick< + KernelStatement, + 'fetchNextBatch' | 'schema' | 'isVolumeOperation' | 'completeVolumeOperation' +>; /** * The rich operation-status surface the kernel exposes on a terminal sync @@ -220,6 +225,8 @@ export default class KernelOperationBackend implements IOperationBackend { // already-terminal statement. Drives both fetch and result-metadata. private fetchHandlePromise?: Promise; + private volumeCompleted = false; + // Async path only: set once the kernel reported the statement Succeeded (a // status poll, or `awaitResult()` resolving). The kernel exposes no // synchronous terminal-state accessor, so the observed success is retained. @@ -256,7 +263,7 @@ export default class KernelOperationBackend implements IOperationBackend { // drop-guard fires whenever this handle is eventually GC'd. this.lifecycleHandle = cancellableExecution ? { - cancel: () => cancellableExecution.cancel(), + cancel: () => (this.blockingStatement ? this.blockingStatement.cancel() : cancellableExecution.cancel()), close: () => (this.blockingStatement ? this.blockingStatement.close() : cancellableExecution.cancel()), } : ((asyncStatement ?? statement) as KernelStatementHandle); @@ -304,6 +311,27 @@ export default class KernelOperationBackend implements IOperationBackend { // Fetch / metadata (owned by the kernel-results pipeline). // --------------------------------------------------------------------------- + public async completeStagingOperation(allowedPaths: string[]): Promise { + failIfNotActive(this.lifecycle); + try { + const handle = await this.getFetchHandle(); + const outcome = await handle.completeVolumeOperation(allowedPaths, true); + if (outcome === 'needs_input_stream') { + throw new StagingError('Volume PUT requires a local file path'); + } + this.volumeCompleted = outcome === 'completed'; + } catch (err) { + const mapped = decodeNapiKernelError(err); + if ( + mapped instanceof ParameterError || + (mapped as { kernelMetadata?: { httpStatus?: number } }).kernelMetadata?.httpStatus + ) { + throw new StagingError(mapped.message); + } + throw mapped; + } + } + public async fetchChunk({ limit, disableBuffering, @@ -316,6 +344,7 @@ export default class KernelOperationBackend implements IOperationBackend { // Cancel-mid-fetch propagation: if cancel() has flipped the // lifecycle flag, fail locally without a wire round-trip. failIfNotActive(this.lifecycle); + if (this.volumeCompleted) return []; // Cooperative cancel (parity with ThriftOperationBackend): the facade // supplies `isClosed` and re-checks `failIfClosed()` after we return, so // bailing with `[]` at a yield point is the contract-correct way for a @@ -356,6 +385,7 @@ export default class KernelOperationBackend implements IOperationBackend { public async hasMore(): Promise { failIfNotActive(this.lifecycle); + if (this.volumeCompleted) return false; const slicer = await this.getResultSlicer(); return slicer.hasMore(); } @@ -394,7 +424,7 @@ export default class KernelOperationBackend implements IOperationBackend { // `arrowSchema` doesn't hit apache-arrow@13's `Unrecognized type // "Duration" (18)`. Matches what the per-batch fetch path already does. arrowSchema: patchIpcBytes(arrowSchemaIpc.ipcBytes), - isStagingOperation: false, + isStagingOperation: Boolean(handle.isVolumeOperation), }; this.metadata = meta; return meta; diff --git a/lib/kernel/KernelSessionBackend.ts b/lib/kernel/KernelSessionBackend.ts index dd55dd4c..92966ac2 100644 --- a/lib/kernel/KernelSessionBackend.ts +++ b/lib/kernel/KernelSessionBackend.ts @@ -151,7 +151,7 @@ export default class KernelSessionBackend implements ISessionBackend { * Still rejected (genuinely unsupported on kernel, rather than silently * dropped): `useCloudFetch` (governed by the kernel `ResultConfig`, not a * per-statement knob), `useLZ4Compression` (kernel owns result compression), - * and `stagingAllowedLocalPath` (volume operations). `maxRows` is applied by + * while `stagingAllowedLocalPath` is applied by the facade. `maxRows` is applied by * the facade at fetch time, so it is intentionally not handled here. */ public async executeStatement(statement: string, options: ExecuteStatementOptions): Promise { @@ -160,8 +160,7 @@ export default class KernelSessionBackend implements ISessionBackend { // Per-statement options the kernel backend doesn't honour are treated as // NO-OPs (logged at warn), not errors — so call sites written for the Thrift // backend are drop-in on the kernel path. These are perf/format hints the - // kernel governs internally (useCloudFetch / useLZ4Compression) or a feature - // it doesn't expose yet (staging); ignoring them cannot change query results. + // kernel governs internally (useCloudFetch / useLZ4Compression). // NB: parameter binding (compound/BINARY) is deliberately NOT no-op'd — a // dropped param would silently change results, so it still throws. if (options.useCloudFetch !== undefined) { @@ -180,15 +179,6 @@ export default class KernelSessionBackend implements ISessionBackend { 'kernel executeStatement: ignoring per-statement `useLZ4Compression` — result compression is governed by the kernel (no-op on kernel).', ); } - if (options.stagingAllowedLocalPath !== undefined) { - this.context - .getLogger() - .log( - LogLevel.warn, - 'kernel executeStatement: ignoring `stagingAllowedLocalPath` — volume/staging operations are not yet supported on the kernel backend (no-op).', - ); - } - // `runAsync` selects the kernel execution path. NOTE: this is a kernel- // specific use of the option — the Thrift backend hardcodes `runAsync: true` // on the wire and never reads `options.runAsync`, so the field is a no-op diff --git a/native/kernel/index.d.ts b/native/kernel/index.d.ts index 1f1d3581..50dd70a6 100644 --- a/native/kernel/index.d.ts +++ b/native/kernel/index.d.ts @@ -18,6 +18,8 @@ * post-close contract. */ export declare class AsyncResultHandle { + get isVolumeOperation(): boolean; + completeVolumeOperation(allowedPaths: Array, removeNotFoundIsSuccess?: boolean | undefined | null): Promise; /** * Server-issued statement id. Cached at construction; readable * for log correlation. Matches the parent `AsyncStatement`'s @@ -432,6 +434,8 @@ export declare class Connection { * disposed statement can still read them. */ export declare class Statement { + get isVolumeOperation(): boolean; + completeVolumeOperation(allowedPaths: Array, removeNotFoundIsSuccess?: boolean | undefined | null): Promise; /** * Server-issued statement id. Cached at construction; readable * even after `close()` so JS-side log lines can correlate against diff --git a/tests/unit/DBSQLSession.test.ts b/tests/unit/DBSQLSession.test.ts index 00498c54..7c73c1c0 100644 --- a/tests/unit/DBSQLSession.test.ts +++ b/tests/unit/DBSQLSession.test.ts @@ -7,6 +7,7 @@ import Status from '../../lib/dto/Status'; import DBSQLOperation from '../../lib/DBSQLOperation'; import { PrimaryKeysRequest } from '../../lib/contracts/IDBSQLSession'; import ISessionBackend from '../../lib/contracts/ISessionBackend'; +import IOperationBackend from '../../lib/contracts/IOperationBackend'; import ParameterError from '../../lib/errors/ParameterError'; import { DBSQLParameter } from '../../lib/DBSQLParameter'; import { TSessionHandle, TProtocolVersion, TOperationState, TStatusCode } from '../../thrift/TCLIService_types'; @@ -883,3 +884,18 @@ describe('DBSQLSession', () => { }); }); }); + +describe('DBSQLSession native staging dispatch', () => { + for (const configured of [false, true]) { + it(`calls native completion only when local paths are ${configured ? 'configured' : 'omitted'}`, async () => { + const completeStagingOperation = sinon.stub().resolves(); + const op = { id: 'volume-operation', completeStagingOperation } as unknown as IOperationBackend; + const session = new DBSQLSession({ + backend: { id: 'volume-session', executeStatement: async () => op } as unknown as ISessionBackend, + context: new ClientContextStub(), + }); + await session.executeStatement('REMOVE remote', configured ? { stagingAllowedLocalPath: [] } : {}); + expect(completeStagingOperation.callCount).to.equal(configured ? 1 : 0); + }); + } +}); diff --git a/tests/unit/kernel/KernelOperationBackend.test.ts b/tests/unit/kernel/KernelOperationBackend.test.ts index 972de4fc..695e82f7 100644 --- a/tests/unit/kernel/KernelOperationBackend.test.ts +++ b/tests/unit/kernel/KernelOperationBackend.test.ts @@ -389,3 +389,45 @@ describe('KernelOperationBackend — M0 datatype round-trip via napi → ArrowRe expect(stub.closed).to.equal(true); }); }); + +describe('KernelOperationBackend Volume completion', () => { + it('routes cancellation to the statement after SQL completes', async () => { + const schema = new Schema([withTypeName(new Field('x', new Int32(), true), 'INT')]); + const statement = new StatementStub(ipcSchemaOnly(schema), []); + let executionCancelled = false; + const execution = { + result: async () => statement, + cancel: async () => { + executionCancelled = true; + }, + }; + const backend = new KernelOperationBackend({ + cancellableExecution: execution as any, + context: new ClientContextStub(), + }); + await backend.getResultMetadata(); + await backend.cancel(); + expect(statement.cancelled).to.equal(true); + expect(executionCancelled).to.equal(false); + }); + + for (const outcome of ['completed', 'not_volume']) { + it(`${ + outcome === 'completed' ? 'consumes completed instructions' : 'preserves raw instruction rows' + }`, async () => { + const schema = new Schema([withTypeName(new Field('x', new Int32(), true), 'INT')]); + const stub = new StatementStub(ipcSchemaOnly(schema), [ipcFromColumns(schema, { x: [1] })]); + const completeVolumeOperation = async (paths: string[], remove404: boolean) => { + expect(paths).to.deep.equal(['/allowed']); + expect(remove404).to.equal(true); + return outcome; + }; + Object.assign(stub, { isVolumeOperation: true, completeVolumeOperation }); + const backend = new KernelOperationBackend({ statement: stub, context: new ClientContextStub() }); + expect((await backend.getResultMetadata()).isStagingOperation).to.equal(true); + await backend.completeStagingOperation(['/allowed']); + expect((await backend.fetchChunk({ limit: 100 })).length).to.equal(outcome === 'completed' ? 0 : 1); + expect(await backend.hasMore()).to.equal(false); + }); + } +}); diff --git a/tests/unit/kernel/error-mapping.test.ts b/tests/unit/kernel/error-mapping.test.ts index fefa8f2a..855c4b58 100644 --- a/tests/unit/kernel/error-mapping.test.ts +++ b/tests/unit/kernel/error-mapping.test.ts @@ -372,3 +372,20 @@ describe('KernelErrorMapping.decodeNapiKernelError — cause chain', () => { expect(err.cause).to.equal(undefined); }); }); + +it('preserves local file error code and path from native completion', () => { + const error = decodeNapiKernelError( + new Error( + `__databricks_error__:${JSON.stringify({ + code: 'NotFound', + message: 'Local file does not exist', + localOsError: 2, + localErrorKind: 'NotFound', + localPath: '/allowed/missing', + })}`, + ), + ) as NodeJS.ErrnoException; + expect(error.code).to.equal('ENOENT'); + expect(error.path).to.equal('/allowed/missing'); + expect(error.errno).to.be.lessThan(0); +}); diff --git a/tests/unit/kernel/execution.test.ts b/tests/unit/kernel/execution.test.ts index 79157f77..0e643f56 100644 --- a/tests/unit/kernel/execution.test.ts +++ b/tests/unit/kernel/execution.test.ts @@ -40,6 +40,12 @@ import { DBSQLParameter, DBSQLParameterType } from '../../../lib/DBSQLParameter' // ----------------------------------------------------------------------------- class FakeNativeStatement implements KernelStatement { + public readonly isVolumeOperation = false; + + public async completeVolumeOperation(): Promise { + return 'not_volume'; + } + public closed = false; public cancelled = false; @@ -1075,13 +1081,11 @@ describe('KernelSessionBackend', () => { // Unsupported-on-kernel per-statement HINTS are NO-OPs (not errors) so Thrift- // targeted call sites are drop-in on the kernel path. They are kernel-governed - // perf/format hints (useCloudFetch / useLZ4Compression) or an unexposed feature - // (staging); ignoring them can't change results. (Parameter binding is NOT here + // perf/format hints (useCloudFetch / useLZ4Compression). (Parameter binding is NOT here // — a dropped param would change results, so it still throws; asserted above.) for (const { name, options } of [ { name: 'useCloudFetch', options: { useCloudFetch: true } }, { name: 'useLZ4Compression', options: { useLZ4Compression: true } }, - { name: 'stagingAllowedLocalPath', options: { stagingAllowedLocalPath: '/tmp' } }, ] as const) { it(`executeStatement ignores unsupported ${name} (no-op, does not throw)`, async () => { const connection = new FakeNativeConnection(); From deb55b6378d97fbf9ec9d1d4e4ac4ef05a5f3dc4 Mon Sep 17 00:00:00 2001 From: Aakash Saravanan Date: Fri, 13 Nov 2026 23:25:05 +0000 Subject: [PATCH 2/8] Retain inline results for configured Volume transfers Signed-off-by: Aakash Saravanan --- lib/contracts/IDBSQLSession.ts | 3 ++- lib/kernel/KernelSessionBackend.ts | 4 +++- tests/unit/kernel/execution.test.ts | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/contracts/IDBSQLSession.ts b/lib/contracts/IDBSQLSession.ts index 12d7d6c2..da5a01b1 100644 --- a/lib/contracts/IDBSQLSession.ts +++ b/lib/contracts/IDBSQLSession.ts @@ -32,7 +32,8 @@ export type ExecuteStatementOptions = { * `false`/unset (default) runs the blocking direct-results path (faster, * cancellable mid-compute); `true` submits and polls (returns a pending * handle before completion). Default is sync, matching the python - * connector's `cursor.execute()`. + * connector's `cursor.execute()`. With `stagingAllowedLocalPath` configured, + * execution waits for staging completion regardless of this option. */ runAsync?: boolean; maxRows?: number | bigint | Int64 | null; diff --git a/lib/kernel/KernelSessionBackend.ts b/lib/kernel/KernelSessionBackend.ts index 92966ac2..a074d4b2 100644 --- a/lib/kernel/KernelSessionBackend.ts +++ b/lib/kernel/KernelSessionBackend.ts @@ -202,7 +202,9 @@ export default class KernelSessionBackend implements ISessionBackend { // the backend polls `status()` to terminal in `waitUntilReady()` and // materialises results via `awaitResult()`. `queryTimeout` is a no-op // here too. - const runAsync = options.runAsync ?? false; + // Configured staging completes before the facade returns, so retain inline + // results even when the caller requests asynchronous execution. + const runAsync = (options.runAsync ?? false) && options.stagingAllowedLocalPath === undefined; // `queryTimeout` is a NO-OP on the kernel (kernel) backend. It is the JDBC // `setQueryTimeout` knob which — per the option's JSDoc — is effective only // on Compute clusters; SQL Warehouses (what kernel targets) use the diff --git a/tests/unit/kernel/execution.test.ts b/tests/unit/kernel/execution.test.ts index 0e643f56..73807700 100644 --- a/tests/unit/kernel/execution.test.ts +++ b/tests/unit/kernel/execution.test.ts @@ -893,6 +893,20 @@ describe('KernelSessionBackend', () => { expect(op.id).to.be.a('string').and.have.length.greaterThan(0); }); + for (const stagingAllowedLocalPath of [[], '/tmp']) { + it(`retains inline results for configured async staging paths ${JSON.stringify( + stagingAllowedLocalPath, + )}`, async () => { + const connection = new FakeNativeConnection(); + const direct = sinon.spy(connection, 'executeStatementDirect'); + const submit = sinon.spy(connection, 'submitStatement'); + const session = makeSession(connection); + await session.executeStatement('REMOVE remote', { runAsync: true, stagingAllowedLocalPath }); + expect(direct.calledOnce).to.equal(true); + expect(submit.called).to.equal(false); + }); + } + 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). From 71547a61bb5933164934f7315de231f51b85158a Mon Sep 17 00:00:00 2001 From: Aakash Saravanan Date: Fri, 13 Nov 2026 23:38:01 +0000 Subject: [PATCH 3/8] Correct Volume backend support documentation Signed-off-by: Aakash Saravanan --- CONNECTION_PARAMETERS.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CONNECTION_PARAMETERS.md b/CONNECTION_PARAMETERS.md index 24ee78de..991bcbbd 100644 --- a/CONNECTION_PARAMETERS.md +++ b/CONNECTION_PARAMETERS.md @@ -228,8 +228,7 @@ session parameter surface. Thrift-only capabilities include custom/token-provider authentication, custom OAuth persistence, SOCKS proxies, authenticated/driver-buffer telemetry knobs, -query timeout, Direct Results `maxRows`, per-statement CloudFetch/LZ4 hints, and -volume operations. +query timeout, Direct Results `maxRows`, and per-statement CloudFetch/LZ4 hints. Kernel-only capabilities include connection-pool sizing, proxy bypass hosts, independent TLS hostname verification, optional U2M disk caching, and From 66d085190f274b8b10ede6d8c9ff6df0863d1282 Mon Sep 17 00:00:00 2001 From: Aakash Saravanan Date: Sat, 14 Nov 2026 00:07:10 +0000 Subject: [PATCH 4/8] Pin the native source for Volume completion Signed-off-by: Aakash Saravanan --- KERNEL_REV | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/KERNEL_REV b/KERNEL_REV index de1b52ca..f85bb468 100644 --- a/KERNEL_REV +++ b/KERNEL_REV @@ -1 +1 @@ -0b5d0edfed32c0745d5abc2468fc3070e2eb2a17 +bdc4d35693ab94ee1991799d12a40503fdf66670 From a3d7ae626bbd73c781a817f61a8b11e8cb7d09af Mon Sep 17 00:00:00 2001 From: Aakash Saravanan Date: Sat, 14 Nov 2026 00:15:38 +0000 Subject: [PATCH 5/8] Select the Volume kernel and format its path option Signed-off-by: Aakash Saravanan --- CONNECTION_PARAMETERS.md | 2 +- KERNEL_REV | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CONNECTION_PARAMETERS.md b/CONNECTION_PARAMETERS.md index 991bcbbd..3d89efc7 100644 --- a/CONNECTION_PARAMETERS.md +++ b/CONNECTION_PARAMETERS.md @@ -197,7 +197,7 @@ telemetry implementation, so only the options explicitly forwarded by | `maxRows` | `number \| bigint \| Int64 \| null` | ✅ | ❌ | `100000` / — | Thrift uses it for the initial Direct Results row count; `null` disables Direct Results. Kernel does not retain or forward the supplied value. | | `useCloudFetch` | `boolean` | ✅ | ❌ | `true` / — | Kernel logs and ignores this per-statement hint; Kernel result fetching owns its CloudFetch behavior. | | `useLZ4Compression` | `boolean` | ✅ | ❌ | `true` / — | Thrift uses it when supported and when the result is not CloudFetch. Kernel owns and auto-detects result compression. | -| `stagingAllowedLocalPath` | `string \| string[]` | ✅ | ✅ | — | Local allowlist for Volume operations. Kernel also requires resolved symlink targets within allowed roots; links within those roots are permitted. | +| `stagingAllowedLocalPath` | `string \| string[]` | ✅ | ✅ | — | Local allowlist for Volume operations. Kernel also requires resolved symlink targets within allowed roots; links within those roots are permitted. | | `namedParameters` | `Record` | ✅ | ✅ | — | Named SQL parameters. Cannot be combined with non-empty `ordinalParameters`. Thrift requires a protocol that supports parameterized queries. | | `ordinalParameters` | `Array` | ✅ | ✅ | — | Positional SQL parameters. Cannot be combined with non-empty `namedParameters`. | | `queryTags` | `Record` | ✅ | ✅ | — | Serialized into the per-statement `query_tags` conf overlay on both. | diff --git a/KERNEL_REV b/KERNEL_REV index f85bb468..a34a99f0 100644 --- a/KERNEL_REV +++ b/KERNEL_REV @@ -1 +1 @@ -bdc4d35693ab94ee1991799d12a40503fdf66670 +c22bd606abc48599ab780a65a1389d141c3414bf From 12876fc7353415a0e8cdb6e7f7371d0168ad38de Mon Sep 17 00:00:00 2001 From: Aakash Saravanan Date: Sat, 14 Nov 2026 00:32:32 +0000 Subject: [PATCH 6/8] Regenerate Volume completion declarations from the pinned kernel Signed-off-by: Aakash Saravanan --- native/kernel/index.d.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/native/kernel/index.d.ts b/native/kernel/index.d.ts index 50dd70a6..a7916f81 100644 --- a/native/kernel/index.d.ts +++ b/native/kernel/index.d.ts @@ -18,8 +18,9 @@ * post-close contract. */ export declare class AsyncResultHandle { - get isVolumeOperation(): boolean; - completeVolumeOperation(allowedPaths: Array, removeNotFoundIsSuccess?: boolean | undefined | null): Promise; + get isVolumeOperation(): boolean + /** Complete a server-marked file transfer while retaining its instruction rows. */ + completeVolumeOperation(allowedPaths: Array, removeNotFoundIsSuccess?: boolean | undefined | null): Promise /** * Server-issued statement id. Cached at construction; readable * for log correlation. Matches the parent `AsyncStatement`'s @@ -434,8 +435,9 @@ export declare class Connection { * disposed statement can still read them. */ export declare class Statement { - get isVolumeOperation(): boolean; - completeVolumeOperation(allowedPaths: Array, removeNotFoundIsSuccess?: boolean | undefined | null): Promise; + get isVolumeOperation(): boolean + /** Complete a server-marked file transfer while retaining its instruction rows. */ + completeVolumeOperation(allowedPaths: Array, removeNotFoundIsSuccess?: boolean | undefined | null): Promise /** * Server-issued statement id. Cached at construction; readable * even after `close()` so JS-side log lines can correlate against From 9c1e0e7ce1b9ed469384875224be7bbda9b85a8d Mon Sep 17 00:00:00 2001 From: Aakash Saravanan Date: Sat, 14 Nov 2026 02:42:18 +0000 Subject: [PATCH 7/8] Preserve SQL execution errors during Volume completion Signed-off-by: Aakash Saravanan --- lib/kernel/KernelOperationBackend.ts | 2 +- tests/unit/DBSQLSession.test.ts | 20 ++++++++ .../kernel/KernelOperationBackend.test.ts | 46 +++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/lib/kernel/KernelOperationBackend.ts b/lib/kernel/KernelOperationBackend.ts index d74f7f60..1b804922 100644 --- a/lib/kernel/KernelOperationBackend.ts +++ b/lib/kernel/KernelOperationBackend.ts @@ -313,8 +313,8 @@ export default class KernelOperationBackend implements IOperationBackend { public async completeStagingOperation(allowedPaths: string[]): Promise { failIfNotActive(this.lifecycle); + const handle = await this.getFetchHandle(); try { - const handle = await this.getFetchHandle(); const outcome = await handle.completeVolumeOperation(allowedPaths, true); if (outcome === 'needs_input_stream') { throw new StagingError('Volume PUT requires a local file path'); diff --git a/tests/unit/DBSQLSession.test.ts b/tests/unit/DBSQLSession.test.ts index 7c73c1c0..505e1b4f 100644 --- a/tests/unit/DBSQLSession.test.ts +++ b/tests/unit/DBSQLSession.test.ts @@ -886,6 +886,26 @@ describe('DBSQLSession', () => { }); describe('DBSQLSession native staging dispatch', () => { + it('closes a failed native operation and preserves its error when close also fails', async () => { + const error = new Error('completion failed'); + const close = sinon.stub().rejects(new Error('close failed')); + const op = { + id: 'volume-operation', + completeStagingOperation: sinon.stub().rejects(error), + close, + } as unknown as IOperationBackend; + const session = new DBSQLSession({ + backend: { id: 'volume-session', executeStatement: async () => op } as unknown as ISessionBackend, + context: new ClientContextStub(), + }); + const actual = await session.executeStatement('GET remote TO local', { stagingAllowedLocalPath: [] }).then( + () => expect.fail('completion should fail'), + (failure) => failure, + ); + expect(actual).to.equal(error); + expect(close.callCount).to.equal(1); + }); + for (const configured of [false, true]) { it(`calls native completion only when local paths are ${configured ? 'configured' : 'omitted'}`, async () => { const completeStagingOperation = sinon.stub().resolves(); diff --git a/tests/unit/kernel/KernelOperationBackend.test.ts b/tests/unit/kernel/KernelOperationBackend.test.ts index 695e82f7..ba34ac45 100644 --- a/tests/unit/kernel/KernelOperationBackend.test.ts +++ b/tests/unit/kernel/KernelOperationBackend.test.ts @@ -40,6 +40,8 @@ import { import KernelOperationBackend from '../../../lib/kernel/KernelOperationBackend'; import { TTypeId } from '../../../thrift/TCLIService_types'; import ClientContextStub from '../.stubs/ClientContextStub'; +import ParameterError from '../../../lib/errors/ParameterError'; +import StagingError from '../../../lib/errors/StagingError'; // Minimal stub of the napi `Statement` surface that emits a precomputed // Arrow IPC payload per `fetchNextBatch()` call. Used to feed @@ -391,6 +393,50 @@ describe('KernelOperationBackend — M0 datatype round-trip via napi → ArrowRe }); describe('KernelOperationBackend Volume completion', () => { + it('preserves SQL execution errors with HTTP metadata', async () => { + const error = Object.assign(new ParameterError('invalid SQL parameter'), { + sqlState: '22023', + kernelMetadata: { httpStatus: 400 }, + }); + const backend = new KernelOperationBackend({ + cancellableExecution: { + result: async () => { + throw error; + }, + } as any, + context: new ClientContextStub(), + }); + const actual = await backend.completeStagingOperation(['/allowed']).then( + () => expect.fail('execution should fail'), + (failure) => failure, + ); + expect(actual).to.equal(error); + expect(actual.sqlState).to.equal('22023'); + }); + + for (const error of [ + new ParameterError('path is not allowed'), + Object.assign(new Error('storage HTTP 403'), { kernelMetadata: { httpStatus: 403 } }), + Object.assign(new Error('source is missing'), { code: 'ENOENT', path: '/allowed/missing' }), + ]) { + it(`classifies transfer failure: ${error.message}`, async () => { + const schema = new Schema([withTypeName(new Field('x', new Int32(), true), 'INT')]); + const statement = new StatementStub(ipcSchemaOnly(schema), []); + Object.assign(statement, { + completeVolumeOperation: async () => { + throw error; + }, + }); + const backend = new KernelOperationBackend({ statement, context: new ClientContextStub() }); + const actual = await backend.completeStagingOperation(['/allowed']).then( + () => expect.fail('transfer should fail'), + (failure) => failure, + ); + if ('code' in error) expect(actual).to.equal(error); + else expect(actual).to.be.instanceOf(StagingError).with.property('message', error.message); + }); + } + it('routes cancellation to the statement after SQL completes', async () => { const schema = new Schema([withTypeName(new Field('x', new Int32(), true), 'INT')]); const statement = new StatementStub(ipcSchemaOnly(schema), []); From 35e025994f78fccee430b1d97c9d0ab5b697d2f2 Mon Sep 17 00:00:00 2001 From: Aakash Saravanan Date: Sat, 14 Nov 2026 02:53:00 +0000 Subject: [PATCH 8/8] Pin native Volume storage retry and terminal response fixes Signed-off-by: Aakash Saravanan --- KERNEL_REV | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/KERNEL_REV b/KERNEL_REV index a34a99f0..8a2f8e59 100644 --- a/KERNEL_REV +++ b/KERNEL_REV @@ -1 +1 @@ -c22bd606abc48599ab780a65a1389d141c3414bf +22f96facc276836bcac7230c8a2c0987f734f2c7