Skip to content

Commit 3f1d903

Browse files
committed
feat(sea): wire named query parameters (:name) through the napi binding
Completes SEA param parity: SeaSessionBackend now forwards `namedParameters` to the kernel via `ExecuteOptions.namedParams` (kernel `param_named` / `NamedTypedValueInput {name, sqlType, value?}`), alongside positional. New `buildSeaNamedParams` reuses the same `toTypedValueInput` mapping (DECIMAL → DECIMAL(p,s), NULL → VOID). Positional and named are mutually exclusive (ParameterError, matching the Thrift backend). Requires kernel napi `namedParams` (extends the positional codec, #84 line). Validated live: named INT/STRING/DECIMAL/multi-param bind; both-forms rejected. 205 sea unit tests pass. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
1 parent 3337234 commit 3f1d903

5 files changed

Lines changed: 85 additions & 27 deletions

File tree

lib/sea/SeaNativeLoader.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,17 @@ export interface SeaNativeTypedValueInput {
8484
value?: string;
8585
}
8686

87+
/**
88+
* A single named bound parameter — a `SeaNativeTypedValueInput` plus its
89+
* `:name`. Maps to the kernel's `param_named`. Built by
90+
* `SeaPositionalParams.buildSeaNamedParams`.
91+
*/
92+
export interface SeaNativeNamedTypedValueInput {
93+
name: string;
94+
sqlType: string;
95+
value?: string;
96+
}
97+
8798
/**
8899
* Per-statement options accepted by the napi `executeStatement`. Matches
89100
* the kernel `ExecuteOptions`. All fields optional; an omitted/empty
@@ -95,6 +106,7 @@ export interface SeaNativeExecuteOptions {
95106
rowLimit?: number;
96107
queryTimeoutSecs?: number;
97108
positionalParams?: Array<SeaNativeTypedValueInput>;
109+
namedParams?: Array<SeaNativeNamedTypedValueInput>;
98110
}
99111

100112
export interface SeaNativeConnection {

lib/sea/SeaPositionalParams.ts

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@
1313
// limitations under the License.
1414

1515
import { DBSQLParameter, DBSQLParameterValue } from '../DBSQLParameter';
16-
import ParameterError from '../errors/ParameterError';
17-
import { SeaNativeTypedValueInput } from './SeaNativeLoader';
16+
import { SeaNativeTypedValueInput, SeaNativeNamedTypedValueInput } from './SeaNativeLoader';
1817

1918
/**
2019
* Derive `(precision,scale)` from a decimal value string for the SEA
@@ -64,13 +63,9 @@ function toTypedValueInput(value: DBSQLParameter | DBSQLParameterValue): SeaNati
6463

6564
/**
6665
* Convert the public `ordinalParameters` option into the napi
67-
* `positionalParams` array. Returns `undefined` when no positional params
68-
* were supplied (so the caller can keep the minimal no-options call shape).
69-
*
70-
* Named parameters are not yet bindable on the SEA path — the kernel napi
71-
* surface (`ExecuteOptions.positionalParams`) exposes positional only — so a
72-
* caller passing `namedParameters` is rejected with a clear `ParameterError`
73-
* rather than silently ignored.
66+
* `positionalParams` array (1-based `?` placeholders). Returns `undefined`
67+
* when none were supplied, so the caller can keep the minimal no-options
68+
* call shape.
7469
*/
7570
export function buildSeaPositionalParams(
7671
ordinalParameters?: Array<DBSQLParameter | DBSQLParameterValue>,
@@ -80,3 +75,21 @@ export function buildSeaPositionalParams(
8075
}
8176
return ordinalParameters.map(toTypedValueInput);
8277
}
78+
79+
/**
80+
* Convert the public `namedParameters` option (`Record<name, value>`) into
81+
* the napi `namedParams` array (`:name` placeholders). Each value reuses the
82+
* same `toTypedValueInput` mapping (DECIMAL → DECIMAL(p,s), NULL → VOID, …),
83+
* then carries its name. Returns `undefined` when none were supplied.
84+
*/
85+
export function buildSeaNamedParams(
86+
namedParameters?: Record<string, DBSQLParameter | DBSQLParameterValue>,
87+
): Array<SeaNativeNamedTypedValueInput> | undefined {
88+
if (namedParameters === undefined || Object.keys(namedParameters).length === 0) {
89+
return undefined;
90+
}
91+
return Object.keys(namedParameters).map((name) => ({
92+
name,
93+
...toTypedValueInput(namedParameters[name]),
94+
}));
95+
}

lib/sea/SeaSessionBackend.ts

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ import { decodeNapiKernelError } from './SeaErrorMapping';
3636
import SeaOperationBackend from './SeaOperationBackend';
3737
import SeaTableTypeFilter from './SeaTableTypeFilter';
3838
import { seaServerInfoValue } from './SeaServerInfo';
39-
import { buildSeaPositionalParams } from './SeaPositionalParams';
39+
import { buildSeaPositionalParams, buildSeaNamedParams } from './SeaPositionalParams';
40+
import ParameterError from '../errors/ParameterError';
4041

4142
export interface SeaSessionBackendOptions {
4243
/** The opaque napi `Connection` handle returned by `openSession`. */
@@ -131,24 +132,23 @@ export default class SeaSessionBackend implements ISessionBackend {
131132
public async executeStatement(statement: string, options: ExecuteStatementOptions): Promise<IOperationBackend> {
132133
this.failIfClosed();
133134

134-
// Named params aren't bindable on the SEA path yet — the kernel napi
135-
// surface (ExecuteOptions.positionalParams) exposes positional only.
136-
// Reject rather than silently ignore.
137-
if (options.namedParameters !== undefined && Object.keys(options.namedParameters).length > 0) {
138-
throw new HiveDriverError(
139-
'SEA executeStatement: named parameters are not supported yet (use positional `?` parameters).',
140-
);
141-
}
142-
143-
// Reduce positional `?` bindings to the napi `{ sqlType, value? }` inputs
144-
// the kernel param codec accepts (DECIMAL → DECIMAL(p,s), NULL →
145-
// value-less), reusing DBSQLParameter's stringification.
135+
// Reduce `?` / `:name` bindings to the napi inputs the kernel param codec
136+
// accepts (DECIMAL → DECIMAL(p,s), NULL → value-less), reusing
137+
// DBSQLParameter's stringification. Positional and named are mutually
138+
// exclusive at the SQL level (matches the Thrift backend).
146139
const positionalParams = buildSeaPositionalParams(options.ordinalParameters);
140+
const namedParams = buildSeaNamedParams(options.namedParameters);
141+
if (positionalParams !== undefined && namedParams !== undefined) {
142+
throw new ParameterError('Driver does not support both ordinal and named parameters.');
143+
}
147144

148145
const nativeOptions: SeaNativeExecuteOptions = {};
149146
if (positionalParams !== undefined) {
150147
nativeOptions.positionalParams = positionalParams;
151148
}
149+
if (namedParams !== undefined) {
150+
nativeOptions.namedParams = namedParams;
151+
}
152152
// JDBC `setQueryTimeout` is whole seconds; the kernel's
153153
// `query_timeout_secs` (SEA wait timeout, on_wait_timeout = CANCEL) is
154154
// the native equivalent. The SEA wire caps it at 50s server-side.

tests/unit/sea/execution.test.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -376,17 +376,25 @@ describe('SeaSessionBackend', () => {
376376
expect(connection.lastOptions?.queryTimeoutSecs).to.equal(30);
377377
});
378378

379-
it('executeStatement still rejects namedParameters (positional only on SEA)', async () => {
379+
it('executeStatement forwards namedParameters as napi namedParams ({name,sqlType,value})', async () => {
380+
const connection = new FakeNativeConnection();
381+
const session = makeSession(connection);
382+
await session.executeStatement('SELECT :x', { namedParameters: { x: 'hello' } });
383+
expect(connection.lastOptions?.namedParams).to.deep.equal([{ name: 'x', sqlType: 'STRING', value: 'hello' }]);
384+
expect(connection.lastOptions?.positionalParams).to.equal(undefined);
385+
});
386+
387+
it('executeStatement rejects mixing ordinal and named parameters', async () => {
380388
const connection = new FakeNativeConnection();
381389
const session = makeSession(connection);
382390
let thrown: unknown;
383391
try {
384-
await session.executeStatement('SELECT :x', { namedParameters: { x: 1 } });
392+
await session.executeStatement('SELECT ? AND :x', { ordinalParameters: [1], namedParameters: { x: 2 } });
385393
} catch (err) {
386394
thrown = err;
387395
}
388-
expect(thrown).to.be.instanceOf(HiveDriverError);
389-
expect((thrown as Error).message).to.match(/named parameters/);
396+
expect(thrown).to.be.instanceOf(Error);
397+
expect((thrown as Error).message).to.match(/both ordinal and named/);
390398
});
391399

392400
it('executeStatement uses the no-options fast path when nothing is bound', async () => {

tests/unit/sea/positionalParams.test.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
// limitations under the License.
1414

1515
import { expect } from 'chai';
16-
import { buildSeaPositionalParams } from '../../../lib/sea/SeaPositionalParams';
16+
import { buildSeaPositionalParams, buildSeaNamedParams } from '../../../lib/sea/SeaPositionalParams';
1717
import { DBSQLParameter, DBSQLParameterType } from '../../../lib/DBSQLParameter';
1818

1919
describe('SeaPositionalParams.buildSeaPositionalParams', () => {
@@ -55,3 +55,28 @@ describe('SeaPositionalParams.buildSeaPositionalParams', () => {
5555
]);
5656
});
5757
});
58+
59+
describe('SeaPositionalParams.buildSeaNamedParams', () => {
60+
it('returns undefined for no named params', () => {
61+
expect(buildSeaNamedParams(undefined)).to.equal(undefined);
62+
expect(buildSeaNamedParams({})).to.equal(undefined);
63+
});
64+
65+
it('emits {name, sqlType, value} triples, reusing the same type mapping', () => {
66+
expect(
67+
buildSeaNamedParams({
68+
n: 42,
69+
s: 'hello',
70+
d: new DBSQLParameter({ type: DBSQLParameterType.DECIMAL, value: '99.99' }),
71+
}),
72+
).to.deep.include.members([
73+
{ name: 'n', sqlType: 'INTEGER', value: '42' },
74+
{ name: 's', sqlType: 'STRING', value: 'hello' },
75+
{ name: 'd', sqlType: 'DECIMAL(4,2)', value: '99.99' },
76+
]);
77+
});
78+
79+
it('maps a named NULL to a value-less VOID input (with the name)', () => {
80+
expect(buildSeaNamedParams({ x: null })).to.deep.equal([{ name: 'x', sqlType: 'VOID' }]);
81+
});
82+
});

0 commit comments

Comments
 (0)