From 56df5296d391f764925f6ac0ca6d62c066645268 Mon Sep 17 00:00:00 2001 From: Cathleen Yan Date: Wed, 26 Aug 2026 01:22:33 +0000 Subject: [PATCH] fix(kernel): preserve qualified interval parameter types Signed-off-by: Cathleen Yan --- CHANGELOG.md | 3 ++ lib/kernel/KernelNativeLoader.ts | 9 ++--- lib/kernel/KernelPositionalParams.ts | 39 +++++----------------- lib/kernel/KernelSessionBackend.ts | 9 +++-- native/kernel/index.d.ts | 38 +++++++++++++++++++++ tests/e2e/kernel/execution-e2e.test.ts | 38 ++++++++++++++++++++- tests/unit/kernel/execution.test.ts | 29 +++++++++++----- tests/unit/kernel/positionalParams.test.ts | 16 ++++++--- 8 files changed, 126 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e4a3aca..8e04f1b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Kernel backend (`useKernel: true`): preserve qualified `INTERVAL MONTH` and + `INTERVAL DAY` parameter types on the SEA wire by using the kernel raw-parameter + path, matching the Go driver. (PECOBLR-4169) - Kernel backend source builds (`useKernel: true`, built from `KERNEL_REV`): `getTypeInfo()` now matches the Thrift backend's canonical 18-column, 20-row type-info result. Customer-facing npm installs require a follow-up bump to a published native package containing this Kernel change. ([databricks-sql-kernel#291](https://github.com/databricks/databricks-sql-kernel/pull/291), PECOBLR-4166) - Kernel backend (`useKernel: true`): **Azure Entra (Azure AD) auth is now threaded through the kernel path.** On `authType: 'databricks-oauth'`: **U2M** (no secret) always routes to `OAuthU2m` — the kernel runs one cloud-blind in-house workspace-federated browser flow (it uses the workspace's OIDC-discovered authorize endpoint verbatim), which works against Azure workspaces, so Azure U2M forwards the in-house app (`databricks-sql-connector`) + `sql offline_access` scopes exactly like AWS/GCP, regardless of `useDatabricksOAuthInAzure` (verified E2E against a live Azure workspace). **M2M** (secret): `useDatabricksOAuthInAzure: true` (or non-Azure) → `OAuthM2m` (workspace-OIDC client-credentials); an Azure host with `useDatabricksOAuthInAzure` absent/`false` → the Entra-direct Azure service-principal M2M (`AzureSpM2m`, the Entra SP creds ride `oauthClientId`/`oauthClientSecret`, `azureTenantId` optional and auto-discovered when omitted). On a non-Azure host `useDatabricksOAuthInAzure` is inert. The `AzureSpM2m` path requires a `databricks-sql-kernel` native module that exposes the Azure SP surface — landed on `main` via [databricks-sql-kernel#282](https://github.com/databricks/databricks-sql-kernel/pull/282) (which the pinned `KERNEL_REV` `ef1a6f2` carries; the surface was originally proposed in [#280](https://github.com/databricks/databricks-sql-kernel/pull/280), which never reached `main`); U2M works on any kernel build. (PECOBLR-4141 / PECOBLR-4120) diff --git a/lib/kernel/KernelNativeLoader.ts b/lib/kernel/KernelNativeLoader.ts index 0b98d156..e2b31b58 100644 --- a/lib/kernel/KernelNativeLoader.ts +++ b/lib/kernel/KernelNativeLoader.ts @@ -36,6 +36,7 @@ import type { ExecuteOptions as NativeExecuteOptions, TypedValueInput as NativeTypedValueInput, NamedTypedValueInput as NativeNamedTypedValueInput, + RawParameterInput as NativeRawParameterInput, AsyncStatement as NativeAsyncStatement, AsyncResultHandle as NativeAsyncResultHandle, CancellableExecution as NativeCancellableExecution, @@ -53,15 +54,11 @@ export type KernelArrowSchema = NativeArrowSchema; export type KernelConnection = NativeConnection; export type KernelStatement = 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`, `statementConf`, `queryTags`). We re-export -// rather than re-declare so the driver-side param codec can never drift from -// the kernel contract. +// Re-export the napi-generated parameter types to stay aligned with the kernel. export type KernelNativeExecuteOptions = NativeExecuteOptions; export type KernelNativeTypedValueInput = NativeTypedValueInput; export type KernelNativeNamedTypedValueInput = NativeNamedTypedValueInput; +export type KernelNativeRawParameterInput = NativeRawParameterInput; // Async-submit surface: `Connection.submitStatement` returns an // `AsyncStatement` (status / awaitResult / cancel / close); `awaitResult` diff --git a/lib/kernel/KernelPositionalParams.ts b/lib/kernel/KernelPositionalParams.ts index 758ec733..34ee2d72 100644 --- a/lib/kernel/KernelPositionalParams.ts +++ b/lib/kernel/KernelPositionalParams.ts @@ -14,7 +14,7 @@ import { DBSQLParameter, DBSQLParameterValue } from '../DBSQLParameter'; import ParameterError from '../errors/ParameterError'; -import { KernelNativeTypedValueInput, KernelNativeNamedTypedValueInput } from './KernelNativeLoader'; +import { KernelNativeRawParameterInput } from './KernelNativeLoader'; import assertBindableValue from './KernelInputValidation'; /** @@ -56,17 +56,8 @@ function decimalPrecisionScale(v: string): string { return `${precision},${scale}`; } -/** - * Reduce a `DBSQLParameter | DBSQLParameterValue` to the napi - * `TypedValueInput` (`{ sqlType, value? }`) the kernel's positional-param - * codec (`parse_typed_value`) accepts. Reuses `DBSQLParameter.toSparkParameter` - * — the same type-inference + value-stringification the Thrift backend uses — - * then adapts the type name to the codec's expectations: - * - DECIMAL → `DECIMAL(p,s)` (parenthesised form required) - * - INTERVAL * → `INTERVAL` (the codec's single interval type name) - * - a missing value ⇒ SQL NULL (`parse_typed_value` maps `value: None` to NULL). - */ -function toTypedValueInput(value: DBSQLParameter | DBSQLParameterValue): KernelNativeTypedValueInput { +/** Convert a parameter to the raw napi shape without dropping SQL type qualifiers. */ +function toRawParameterInput(value: DBSQLParameter | DBSQLParameterValue): KernelNativeRawParameterInput { const param = value instanceof DBSQLParameter ? value : new DBSQLParameter({ value }); const spark = param.toSparkParameter(); const stringValue = spark.value?.stringValue ?? undefined; @@ -81,44 +72,32 @@ function toTypedValueInput(value: DBSQLParameter | DBSQLParameterValue): KernelN const upper = sqlType.toUpperCase(); if (upper === 'DECIMAL') { sqlType = `DECIMAL(${decimalPrecisionScale(stringValue)})`; - } else if (upper.startsWith('INTERVAL')) { - sqlType = 'INTERVAL'; } return { sqlType, value: stringValue }; } -/** - * Convert the public `ordinalParameters` option into the napi - * `positionalParams` array (1-based `?` placeholders). Returns `undefined` - * when none were supplied, so the caller can keep the minimal no-options - * call shape. - */ +/** Build positional raw parameters; the kernel assigns their 1-based ordinals. */ export function buildKernelPositionalParams( ordinalParameters?: Array, -): Array | undefined { +): Array | undefined { if (ordinalParameters === undefined || ordinalParameters.length === 0) { return undefined; } return ordinalParameters.map((value, i) => { assertBindableValue(value, `ordinalParameters[${i}]`); - return toTypedValueInput(value); + return toRawParameterInput(value); }); } -/** - * Convert the public `namedParameters` option (`Record`) into - * the napi `namedParams` array (`:name` placeholders). Each value reuses the - * same `toTypedValueInput` mapping (DECIMAL → DECIMAL(p,s), NULL → VOID, …), - * then carries its name. Returns `undefined` when none were supplied. - */ +/** Build named raw parameters while preserving marker names. */ export function buildKernelNamedParams( namedParameters?: Record, -): Array | undefined { +): Array | undefined { if (namedParameters === undefined || Object.keys(namedParameters).length === 0) { return undefined; } return Object.keys(namedParameters).map((name) => { assertBindableValue(namedParameters[name], `namedParameters[${name}]`); - return { name, ...toTypedValueInput(namedParameters[name]) }; + return { name, ...toRawParameterInput(namedParameters[name]) }; }); } diff --git a/lib/kernel/KernelSessionBackend.ts b/lib/kernel/KernelSessionBackend.ts index 84a765cb..6cbc745e 100644 --- a/lib/kernel/KernelSessionBackend.ts +++ b/lib/kernel/KernelSessionBackend.ts @@ -298,11 +298,10 @@ export default class KernelSessionBackend implements ISessionBackend { } const execOptions: KernelNativeExecuteOptions = {}; - if (positionalParams !== undefined) { - execOptions.positionalParams = positionalParams; - } - if (namedParams !== undefined) { - execOptions.namedParams = namedParams; + // Raw binding preserves qualified SQL types such as INTERVAL MONTH. + const rawParams = positionalParams ?? namedParams; + if (rawParams !== undefined) { + execOptions.rawParams = rawParams; } // NB: `queryTimeout` is intentionally NOT forwarded — it is a no-op on kernel // (SQL Warehouses use `STATEMENT_TIMEOUT`; mapping it to `wait_timeout` would diff --git a/native/kernel/index.d.ts b/native/kernel/index.d.ts index 6fb596e2..7006a09a 100644 --- a/native/kernel/index.d.ts +++ b/native/kernel/index.d.ts @@ -1034,6 +1034,8 @@ export interface ConnectionOptions { * `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`. + * `rawParams` carries pre-marshalled parameters whose SQL type must be + * preserved verbatim on the SEA wire (for example `INTERVAL MONTH`). * (There is no `queryTimeoutSecs`: it abused the SEA `wait_timeout` inline-hold * window and was removed — a real per-statement timeout is `STATEMENT_TIMEOUT`.) * @@ -1099,6 +1101,19 @@ export interface ExecuteOptions { * mutually exclusive at the SQL level (`?` vs `:name`). */ namedParams?: Array + /** + * Pre-marshalled SEA parameters. The SQL type and string value are sent + * verbatim through `StatementSpec::param_raw`, bypassing `TypedValue` + * conversion. This preserves qualified types such as `INTERVAL MONTH` + * and `INTERVAL DAY` that the generic `TypedValue::Interval` cannot + * represent. + * + * Omit `name` for positional parameters (ordinals are assigned 1-based + * in array order); include it for named parameters. Raw parameters are + * mutually exclusive with `positionalParams` / `namedParams`, and a raw + * list cannot mix named and positional markers. + */ + rawParams?: Array } /** @@ -1248,6 +1263,29 @@ export interface TypedValueInput { value?: string } +/** + * JS-visible pre-marshalled SEA parameter. + * + * Unlike [`TypedValueInput`], this shape bypasses the kernel's + * `(sql_type, value) -> TypedValue` codec and preserves `sql_type` verbatim on + * the SEA wire. This is required for qualified types such as + * `INTERVAL MONTH` and `INTERVAL DAY`, whose qualifier cannot be represented + * by the kernel's generic `TypedValue::Interval` variant. + * + * `name: None` denotes a positional marker; the kernel assigns 1-based + * ordinals in array order. `name: Some(...)` denotes a named marker. A single + * statement must not mix named and positional raw parameters, or raw and + * typed parameters; `StatementSpec` enforces both rules before dispatch. + */ +export interface RawParameterInput { + /** Named marker name. Omit for a positional marker. */ + name?: string + /** Databricks SQL type name sent verbatim to SEA. */ + sqlType: string + /** String-encoded value. `None` represents SQL NULL. */ + value?: string +} + /** * Returns the native binding's crate version (`CARGO_PKG_VERSION`). * diff --git a/tests/e2e/kernel/execution-e2e.test.ts b/tests/e2e/kernel/execution-e2e.test.ts index d1ffc9ba..44881bda 100644 --- a/tests/e2e/kernel/execution-e2e.test.ts +++ b/tests/e2e/kernel/execution-e2e.test.ts @@ -13,7 +13,7 @@ // limitations under the License. import { expect } from 'chai'; -import { DBSQLClient } from '../../../lib'; +import { DBSQLClient, DBSQLParameter, DBSQLParameterType } from '../../../lib'; import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient'; import { InternalConnectionOptions } from '../../../lib/contracts/InternalConnectionOptions'; @@ -121,4 +121,40 @@ describe('kernel execution end-to-end', function e2eSuite() { await session.close(); await client.close(); }); + + it('preserves INTERVAL MONTH on the SEA wire', async () => { + const client = new DBSQLClient(); + + await client.connect({ + host: hostName as string, + path: httpPath as string, + token: token as string, + useKernel: true, + } as ConnectionOptions & InternalConnectionOptions); + + const session = await client.openSession({ initialCatalog: 'main' }); + let operation; + let caught: unknown; + try { + operation = await session.executeStatement('SELECT ?', { + ordinalParameters: [ + new DBSQLParameter({ + type: DBSQLParameterType.INTERVALMONTH, + value: '2-6', + }), + ], + }); + await operation.fetchAll(); + } catch (error) { + caught = error; + } finally { + await operation?.close(); + await session.close(); + await client.close(); + } + + // "2-6" is valid YEAR TO MONTH syntax, but invalid for INTERVAL MONTH. + expect(caught).to.be.instanceOf(Error); + expect((caught as Error & { sqlState?: string }).sqlState).to.equal('22023'); + }); }); diff --git a/tests/unit/kernel/execution.test.ts b/tests/unit/kernel/execution.test.ts index 101d698a..bec53033 100644 --- a/tests/unit/kernel/execution.test.ts +++ b/tests/unit/kernel/execution.test.ts @@ -27,6 +27,7 @@ import ParameterError from '../../../lib/errors/ParameterError'; import OperationStateError, { OperationStateErrorCode } from '../../../lib/errors/OperationStateError'; import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient'; import { OperationState } from '../../../lib/contracts/OperationStatus'; +import { DBSQLParameter, DBSQLParameterType } from '../../../lib/DBSQLParameter'; // ----------------------------------------------------------------------------- // Fakes — minimal stand-ins for the napi-rs generated surface and the @@ -708,26 +709,36 @@ describe('KernelSessionBackend', () => { expect(connection.statementToReturn.cancelled, 'cancel reaches the terminal statement').to.equal(true); }); - it('executeStatement forwards ordinalParameters as napi positionalParams', async () => { + it('executeStatement forwards ordinalParameters through napi rawParams', async () => { const connection = new FakeNativeConnection(); const session = makeSession(connection); await session.executeStatement('SELECT ?', { ordinalParameters: [42, 'hi'] }); - const options = connection.lastOptions as { positionalParams?: Array<{ sqlType: string; value?: string }> }; + const options = connection.lastOptions as { rawParams?: Array<{ sqlType: string; value?: string }> }; expect(options, 'options should be passed').to.not.equal(undefined); - expect(options.positionalParams).to.have.length(2); - expect(options.positionalParams?.[0]).to.deep.equal({ sqlType: 'INTEGER', value: '42' }); - expect(options.positionalParams?.[1]).to.deep.equal({ sqlType: 'STRING', value: 'hi' }); + expect(options.rawParams).to.have.length(2); + expect(options.rawParams?.[0]).to.deep.equal({ sqlType: 'INTEGER', value: '42' }); + expect(options.rawParams?.[1]).to.deep.equal({ sqlType: 'STRING', value: 'hi' }); }); - it('executeStatement forwards namedParameters as napi namedParams (:name carried)', async () => { + it('executeStatement forwards namedParameters through napi rawParams (:name carried)', async () => { const connection = new FakeNativeConnection(); const session = makeSession(connection); await session.executeStatement('SELECT :x', { namedParameters: { x: 7 } }); const options = connection.lastOptions as { - namedParams?: Array<{ name: string; sqlType: string; value?: string }>; + rawParams?: Array<{ name?: string; sqlType: string; value?: string }>; }; - expect(options.namedParams).to.have.length(1); - expect(options.namedParams?.[0]).to.deep.equal({ name: 'x', sqlType: 'INTEGER', value: '7' }); + expect(options.rawParams).to.have.length(1); + expect(options.rawParams?.[0]).to.deep.equal({ name: 'x', sqlType: 'INTEGER', value: '7' }); + }); + + it('executeStatement preserves a qualified INTERVAL type in napi rawParams', async () => { + const connection = new FakeNativeConnection(); + const session = makeSession(connection); + await session.executeStatement('SELECT ?', { + ordinalParameters: [new DBSQLParameter({ type: DBSQLParameterType.INTERVALMONTH, value: '2-6' })], + }); + const options = connection.lastOptions as { rawParams?: Array<{ sqlType: string; value?: string }> }; + expect(options.rawParams).to.deep.equal([{ sqlType: 'INTERVAL MONTH', value: '2-6' }]); }); it('executeStatement sends no options object on the no-params path', async () => { diff --git a/tests/unit/kernel/positionalParams.test.ts b/tests/unit/kernel/positionalParams.test.ts index f6070147..960b2689 100644 --- a/tests/unit/kernel/positionalParams.test.ts +++ b/tests/unit/kernel/positionalParams.test.ts @@ -63,15 +63,15 @@ describe('KernelPositionalParams.buildKernelPositionalParams', () => { expect(decimal('')).to.throw(ParameterError, /not a plain decimal numeral/); }); - it('collapses every INTERVAL subtype to the kernel codec\'s single "INTERVAL" type name', () => { + it('preserves qualified INTERVAL types for the kernel raw binder', () => { expect( buildKernelPositionalParams([ - new DBSQLParameter({ type: DBSQLParameterType.INTERVALMONTH, value: '13' }), + new DBSQLParameter({ type: DBSQLParameterType.INTERVALMONTH, value: '2-6' }), new DBSQLParameter({ type: DBSQLParameterType.INTERVALDAY, value: '1 02:03:04' }), ]), ).to.deep.equal([ - { sqlType: 'INTERVAL', value: '13' }, - { sqlType: 'INTERVAL', value: '1 02:03:04' }, + { sqlType: 'INTERVAL MONTH', value: '2-6' }, + { sqlType: 'INTERVAL DAY', value: '1 02:03:04' }, ]); }); @@ -127,4 +127,12 @@ describe('KernelPositionalParams.buildKernelNamedParams', () => { it('maps a named NULL to a value-less VOID input (with the name)', () => { expect(buildKernelNamedParams({ x: null })).to.deep.equal([{ name: 'x', sqlType: 'VOID' }]); }); + + it('preserves a named qualified INTERVAL type', () => { + expect( + buildKernelNamedParams({ + duration: new DBSQLParameter({ type: DBSQLParameterType.INTERVALMONTH, value: '2-6' }), + }), + ).to.deep.equal([{ name: 'duration', sqlType: 'INTERVAL MONTH', value: '2-6' }]); + }); });