From 87e0a30cd222e53b4411c0204eb74b37cd3b8f20 Mon Sep 17 00:00:00 2001 From: Madhavendra Rathore Date: Fri, 29 May 2026 06:38:15 +0000 Subject: [PATCH] feat(sea): TypedValue codec converter for positional params Add toNapiTypedValue() and convertOrdinalParametersToTypedValueInputs() helpers that translate the Node driver's user-facing { ordinalParameters: [...] } shape into the flat { sqlType, value: string | null } payload the SEA napi codec (databricks-sql-kernel/napi/src/params.rs) consumes. The stringification rules (boolean -> "TRUE"/"FALSE", Date -> toISOString(), BigInt/Int64 -> .toString(), integer Number -> "INTEGER", non-integer Number -> "DOUBLE") are kept in lock-step with the Thrift emitter's toSparkParameter, so a positional value bound on either backend hits the server with the same wire-level (type-name, value) pair. A regression-lock test pins this alignment branch-by-branch. This is the JS-side half of the C5 (params-basic-binding) cluster. The kernel-napi half lands in databricks-sql-kernel PR #84. SEA-side wiring of these helpers into Connection.executeStatement(sql, options) happens once the SEA backend abstraction (PR #378) lands on main. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore --- lib/DBSQLParameter.ts | 127 +++++++++++++++++++++++ tests/unit/DBSQLParameter.test.ts | 165 +++++++++++++++++++++++++++++- 2 files changed, 291 insertions(+), 1 deletion(-) diff --git a/lib/DBSQLParameter.ts b/lib/DBSQLParameter.ts index 5e7f0abc..8e01ed05 100644 --- a/lib/DBSQLParameter.ts +++ b/lib/DBSQLParameter.ts @@ -3,6 +3,37 @@ import { TSparkParameter, TSparkParameterValue } from '../thrift/TCLIService_typ export type DBSQLParameterValue = undefined | null | boolean | number | bigint | Int64 | Date | string; +/** + * Wire shape expected by the SEA napi codec + * (`databricks-sql-kernel/napi/src/params.rs::TypedValueInput`). The Rust + * side parses `value` per `sqlType`; we stringify the JS value the same + * way `toSparkParameter` does for Thrift, then hand the pair to napi. + * + * Why a parallel converter rather than re-using `toSparkParameter` and + * unwrapping the wire-Thrift `TSparkParameter`: napi consumes a flat + * `{ sqlType, value: string | null }` POJO. Mining the value out of a + * `TSparkParameter` (which wraps it in `TSparkParameterValue.stringValue` + * and may use `name` for named-binding mode) requires more glue than + * just emitting the SEA shape directly. The two emitters share the + * same stringification rules — boolean → "TRUE"/"FALSE", Date → + * `toISOString()`, etc. + */ +export interface TypedValueInput { + /** + * Canonical Databricks SQL type name (`"INT"`, `"STRING"`, + * `"DECIMAL(10,2)"`, …). The napi codec is case-insensitive for the + * simple variants and requires the parenthesised form for DECIMAL. + */ + sqlType: string; + /** + * String-encoded literal, or `null` for SQL NULL. The Rust codec + * short-circuits to `TypedValue::Null` regardless of `sqlType` when + * this is `null` — matches the Thrift `TSparkParameter(name)` (no + * type, no value) shape on the wire. + */ + value: string | null; +} + export enum DBSQLParameterType { VOID = 'VOID', // aka NULL STRING = 'STRING', @@ -98,4 +129,100 @@ export class DBSQLParameter { }), }); } + + /** + * SEA-backend sibling of `toSparkParameter`. Emits the flat + * `{ sqlType, value }` shape the napi codec consumes. + * + * Stringification rules are kept in lock-step with `toSparkParameter` + * so a positional parameter bound on the Thrift backend and on the + * SEA backend hits the server with the same wire-level type-name + + * value string. Divergence between the two emitters here would + * silently re-introduce the kind of cross-backend behavior split the + * driver-test parity suite is built to catch. + * + * @returns null for SQL NULL (caller is expected to emit it as + * `{ sqlType: "VOID", value: null }` or to skip the entry, + * depending on call-site convention). This method itself never + * throws; unsupported value shapes fall through the STRING default + * to match the Thrift emitter's behaviour. + */ + public toNapiTypedValue(): TypedValueInput { + // VOID literal — explicit NULL, the napi codec short-circuits. + if (this.type === DBSQLParameterType.VOID) { + return { sqlType: 'VOID', value: null }; + } + + // Inferred NULL — same shape as VOID. The napi codec's contract is + // that `value: null` produces `TypedValue::Null` regardless of + // sqlType, so any non-empty sqlType would work here; we emit VOID + // for consistency with the explicit-NULL path above. + if (this.value === undefined || this.value === null) { + return { sqlType: 'VOID', value: null }; + } + + if (typeof this.value === 'boolean') { + return { + sqlType: this.type ?? DBSQLParameterType.BOOLEAN, + // Thrift emits "TRUE" / "FALSE"; the napi `parse_bool` accepts + // both "true"/"false" and "TRUE"/"FALSE" via its case-insensitive + // match. Keep the casing aligned with the Thrift emitter so any + // log scrape that diffs the two wires sees identical strings. + value: this.value ? 'TRUE' : 'FALSE', + }; + } + + if (typeof this.value === 'number') { + return { + sqlType: this.type ?? (Number.isInteger(this.value) ? DBSQLParameterType.INTEGER : DBSQLParameterType.DOUBLE), + value: Number(this.value).toString(), + }; + } + + if (this.value instanceof Int64 || typeof this.value === 'bigint') { + return { + sqlType: this.type ?? DBSQLParameterType.BIGINT, + value: this.value.toString(), + }; + } + + if (this.value instanceof Date) { + return { + sqlType: this.type ?? DBSQLParameterType.TIMESTAMP, + value: this.value.toISOString(), + }; + } + + return { + sqlType: this.type ?? DBSQLParameterType.STRING, + value: this.value, + }; + } +} + +/** + * Convert the user-facing `ordinalParameters` array into the flat + * `TypedValueInput[]` shape the SEA napi codec accepts. + * + * Mirrors the ordinal-only branch of `lib/DBSQLSession.ts::getQueryParameters` + * — entries may be a `DBSQLParameter` instance or a bare value, and a + * bare value is wrapped in a `DBSQLParameter` for emission. The wrapping + * path is the load-bearing one (today the Node driver's public surface + * accepts bare JS values for positional binding); this helper is the + * single source of truth for how those bare values stringify against + * the napi codec. + * + * Returns an empty array for an undefined / empty input. The caller is + * expected to fall back to a no-positional-params execute in that case. + */ +export function convertOrdinalParametersToTypedValueInputs( + ordinalParameters?: Array, +): Array { + if (ordinalParameters === undefined || ordinalParameters.length === 0) { + return []; + } + return ordinalParameters.map((value) => { + const param = value instanceof DBSQLParameter ? value : new DBSQLParameter({ value }); + return param.toNapiTypedValue(); + }); } diff --git a/tests/unit/DBSQLParameter.test.ts b/tests/unit/DBSQLParameter.test.ts index a3f7659e..7eaf2708 100644 --- a/tests/unit/DBSQLParameter.test.ts +++ b/tests/unit/DBSQLParameter.test.ts @@ -1,7 +1,13 @@ import { expect } from 'chai'; import Int64 from 'node-int64'; import { TSparkParameterValue, TSparkParameter } from '../../thrift/TCLIService_types'; -import { DBSQLParameter, DBSQLParameterType, DBSQLParameterValue } from '../../lib/DBSQLParameter'; +import { + DBSQLParameter, + DBSQLParameterType, + DBSQLParameterValue, + TypedValueInput, + convertOrdinalParametersToTypedValueInputs, +} from '../../lib/DBSQLParameter'; describe('DBSQLParameter', () => { it('should infer types correctly', () => { @@ -101,4 +107,161 @@ describe('DBSQLParameter', () => { expect(dbsqlParam.toSparkParameter()).to.deep.equal(expectedParam); } }); + + // ─── toNapiTypedValue (SEA-backend codec input) ───────────────────── + // + // These tests pin the wire-level alignment between the Thrift emitter + // (`toSparkParameter`) and the SEA emitter (`toNapiTypedValue`). + // Divergence between the two is a parity regression — see C5 cluster + // in sea-workflow/decisions/2026-05-28-autonomous-parity-decisions.md. + + describe('toNapiTypedValue', () => { + it('infers types in sync with toSparkParameter (10-type C5 matrix)', () => { + const cases: Array<[DBSQLParameterValue, TypedValueInput]> = [ + // BOOLEAN — note "TRUE"/"FALSE" casing matches Thrift; the napi + // codec's parse_bool is case-insensitive so this is wire-safe. + [false, { sqlType: DBSQLParameterType.BOOLEAN, value: 'FALSE' }], + [true, { sqlType: DBSQLParameterType.BOOLEAN, value: 'TRUE' }], + // INTEGER from a plain JS number (`Number.isInteger(123) === true`). + [123, { sqlType: DBSQLParameterType.INTEGER, value: '123' }], + // DOUBLE from a non-integer JS number. + [3.14, { sqlType: DBSQLParameterType.DOUBLE, value: '3.14' }], + // BIGINT from JS bigint. + [BigInt(9999999999), { sqlType: DBSQLParameterType.BIGINT, value: '9999999999' }], + // BIGINT from Int64 (the node-int64 path the Thrift driver uses). + [new Int64(1234), { sqlType: DBSQLParameterType.BIGINT, value: '1234' }], + // TIMESTAMP from a JS Date — `.toISOString()` emits the trailing + // `Z`, the napi codec strips it before NaiveDateTime parsing. + [ + new Date('2023-09-06T03:14:27.843Z'), + { sqlType: DBSQLParameterType.TIMESTAMP, value: '2023-09-06T03:14:27.843Z' }, + ], + // STRING fallback. + ['Hello', { sqlType: DBSQLParameterType.STRING, value: 'Hello' }], + ]; + for (const [value, expected] of cases) { + const param = new DBSQLParameter({ value }); + expect(param.toNapiTypedValue()).to.deep.equal(expected); + } + }); + + it('honours an explicitly-set type', () => { + const customType = 'DECIMAL(10,2)' as DBSQLParameterType; + const param = new DBSQLParameter({ type: customType, value: '-123.45' }); + expect(param.toNapiTypedValue()).to.deep.equal({ + sqlType: 'DECIMAL(10,2)', + value: '-123.45', + }); + }); + + it('emits VOID/null for an explicit VOID type regardless of value', () => { + const param = new DBSQLParameter({ type: DBSQLParameterType.VOID, value: 'ignored' }); + expect(param.toNapiTypedValue()).to.deep.equal({ sqlType: 'VOID', value: null }); + }); + + it('emits VOID/null for an undefined or null value', () => { + for (const value of [undefined, null] as Array) { + const param = new DBSQLParameter({ value }); + expect(param.toNapiTypedValue()).to.deep.equal({ sqlType: 'VOID', value: null }); + } + }); + + it('alignment regression-lock: every type-inference branch matches the Thrift emitter', () => { + // Every value the Thrift emitter handles must round-trip to the + // same wire-level (sqlType, stringValue) pair through the napi + // emitter. This is the one-liner that catches a future + // refactor that diverges the two stringification paths. + const values: Array = [ + false, + true, + 0, + 1, + -1, + 3.14, + BigInt(0), + new Int64(0), + new Date(0), + '', + ]; + for (const value of values) { + const param = new DBSQLParameter({ value }); + const thrift = param.toSparkParameter(); + const napi = param.toNapiTypedValue(); + expect(napi.sqlType, `sqlType for ${String(value)}`).to.equal(thrift.type); + expect(napi.value, `value for ${String(value)}`).to.equal(thrift.value?.stringValue); + } + }); + }); + + describe('convertOrdinalParametersToTypedValueInputs', () => { + it('returns [] for undefined input', () => { + expect(convertOrdinalParametersToTypedValueInputs(undefined)).to.deep.equal([]); + }); + + it('returns [] for an empty array', () => { + expect(convertOrdinalParametersToTypedValueInputs([])).to.deep.equal([]); + }); + + it('wraps bare JS values in a DBSQLParameter for stringification', () => { + const got = convertOrdinalParametersToTypedValueInputs([42, 'hello', true, null]); + expect(got).to.deep.equal([ + { sqlType: DBSQLParameterType.INTEGER, value: '42' }, + { sqlType: DBSQLParameterType.STRING, value: 'hello' }, + { sqlType: DBSQLParameterType.BOOLEAN, value: 'TRUE' }, + // null → VOID/null per the toNapiTypedValue contract. + { sqlType: 'VOID', value: null }, + ]); + }); + + it('passes DBSQLParameter instances through with their explicit type', () => { + const decimal = new DBSQLParameter({ + type: 'DECIMAL(10,2)' as DBSQLParameterType, + value: '-123.45', + }); + const got = convertOrdinalParametersToTypedValueInputs([decimal, BigInt('9999999999')]); + expect(got).to.deep.equal([ + { sqlType: 'DECIMAL(10,2)', value: '-123.45' }, + { sqlType: DBSQLParameterType.BIGINT, value: '9999999999' }, + ]); + }); + + it('preserves order — index i in the input is index i on the wire', () => { + // The napi codec's `positional_params` is 1-based (index i ↔ + // ordinal i+1). The JS adapter is responsible for preserving the + // input ordering — this test pins that contract. + const got = convertOrdinalParametersToTypedValueInputs([1, 2, 3, 4, 5]); + expect(got.map((x) => x.value)).to.deep.equal(['1', '2', '3', '4', '5']); + }); + + it('c5 ten-type matrix — one positional value per basic SQL type', () => { + // Mirrors the kernel-napi `c5_ten_type_matrix_round_trips` test + // (databricks-sql-kernel/napi/src/params.rs) one-for-one. The + // wire-level (sqlType, value) pairs emitted here must be the + // exact input the kernel codec accepts. BINARY is omitted — + // the napi codec rejects it at bind time, see the cross-driver + // validation reply from python-sea-oracle for the contract. + const inputs: Array = [ + new DBSQLParameter({ type: DBSQLParameterType.INTEGER, value: 42 }), + new DBSQLParameter({ type: DBSQLParameterType.BIGINT, value: BigInt('9999999999') }), + new DBSQLParameter({ type: DBSQLParameterType.FLOAT, value: 3.14 }), + new DBSQLParameter({ type: DBSQLParameterType.DOUBLE, value: 2.718281828459045 }), + new DBSQLParameter({ type: DBSQLParameterType.STRING, value: 'hello world' }), + new DBSQLParameter({ type: DBSQLParameterType.BOOLEAN, value: true }), + new DBSQLParameter({ type: DBSQLParameterType.DATE, value: '2026-05-15' }), + new DBSQLParameter({ type: DBSQLParameterType.TIMESTAMP, value: '2026-05-15 12:30:45' }), + new DBSQLParameter({ type: 'DECIMAL(10,2)' as DBSQLParameterType, value: '-123.45' }), + ]; + const got = convertOrdinalParametersToTypedValueInputs(inputs); + expect(got).to.have.lengthOf(9); + expect(got[0]).to.deep.equal({ sqlType: 'INTEGER', value: '42' }); + expect(got[1]).to.deep.equal({ sqlType: 'BIGINT', value: '9999999999' }); + expect(got[2]).to.deep.equal({ sqlType: 'FLOAT', value: '3.14' }); + expect(got[3]).to.deep.equal({ sqlType: 'DOUBLE', value: '2.718281828459045' }); + expect(got[4]).to.deep.equal({ sqlType: 'STRING', value: 'hello world' }); + expect(got[5]).to.deep.equal({ sqlType: 'BOOLEAN', value: 'TRUE' }); + expect(got[6]).to.deep.equal({ sqlType: 'DATE', value: '2026-05-15' }); + expect(got[7]).to.deep.equal({ sqlType: 'TIMESTAMP', value: '2026-05-15 12:30:45' }); + expect(got[8]).to.deep.equal({ sqlType: 'DECIMAL(10,2)', value: '-123.45' }); + }); + }); });