Skip to content

Commit e09869b

Browse files
committed
feat(sea): wire rowLimit, statementConf, queryTags + TIMESTAMP_NTZ/LTZ params
Four statement-option / param-type gaps where the kernel + napi were already ready but the node SEA layer didn't expose or thread them, so they were silently dropped vs the Thrift backend. - queryTags: `ExecuteStatementOptions.queryTags` is a public option but `SeaSessionBackend.executeStatement` never forwarded it. Now serialised JS-side via `serializeQueryTags` into the conf overlay's `query_tags` key (the same wire shape the Thrift backend produces), so null-valued tags round-trip — the napi `queryTags` field is a `HashMap<String,String>` that can't represent nulls, and the kernel rejects setting both. - rowLimit: new `ExecuteStatementOptions.rowLimit` → napi `rowLimit` (SEA `row_limit`). SEA-only server-side cap; Thrift has no execute-time cap. - statementConf: new `ExecuteStatementOptions.statementConf` → napi `statementConf` (SEA `statement_conf`), the Thrift `confOverlay` equivalent; queryTags merge into it under `query_tags`. - TIMESTAMP_NTZ / TIMESTAMP_LTZ: added to `DBSQLParameterType` so callers can bind timezone-explicit timestamp params. `toSparkParameter` already honours an explicit type, and `SeaPositionalParams` passes the SQL type verbatim to the kernel codec (which has the NTZ/LTZ arms). Without these a migrating caller silently coerces NTZ/LTZ columns to TIMESTAMP. 231 SEA unit tests pass; verified live against pecotesting (rowLimit caps rows, NTZ param round-trips, tags/conf accepted). Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
1 parent f857d15 commit e09869b

5 files changed

Lines changed: 117 additions & 12 deletions

File tree

lib/DBSQLParameter.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@ export enum DBSQLParameterType {
88
STRING = 'STRING',
99
DATE = 'DATE',
1010
TIMESTAMP = 'TIMESTAMP',
11+
// Timezone-explicit timestamp variants. A bare `Date` value defaults to
12+
// `TIMESTAMP`; set one of these explicitly to bind a TIMESTAMP_NTZ
13+
// (no timezone, wall-clock) or TIMESTAMP_LTZ (local timezone) parameter.
14+
// The Thrift wire only has `TIMESTAMP`; these are SEA-path types the kernel
15+
// param codec accepts — without them a migrating caller silently coerces
16+
// NTZ/LTZ columns to TIMESTAMP.
17+
TIMESTAMP_NTZ = 'TIMESTAMP_NTZ',
18+
TIMESTAMP_LTZ = 'TIMESTAMP_LTZ',
1119
FLOAT = 'FLOAT',
1220
DECIMAL = 'DECIMAL',
1321
DOUBLE = 'DOUBLE',

lib/contracts/IDBSQLSession.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,20 @@ export type ExecuteStatementOptions = {
2727
* These tags apply only to this statement and do not persist across queries.
2828
*/
2929
queryTags?: Record<string, string | null | undefined>;
30+
/**
31+
* Server-side cap on the number of rows the statement returns (SEA path only).
32+
* Maps to the kernel's `row_limit` / SEA `row_limit`. The Thrift backend has no
33+
* execute-time server cap, so this is a no-op there; use `maxRows` for the
34+
* client-side per-fetch chunk size on both backends.
35+
*/
36+
rowLimit?: number;
37+
/**
38+
* Arbitrary per-statement configuration overlay (SEA path only). Maps to the
39+
* kernel's `statement_conf` / SEA `statement_conf`, the same mechanism the
40+
* Thrift backend exposes as `confOverlay`. `queryTags` are merged into this map
41+
* under the `query_tags` key, mirroring the Thrift wire shape.
42+
*/
43+
statementConf?: Record<string, string>;
3044
};
3145

3246
export type TypeInfoRequest = {

lib/sea/SeaSessionBackend.ts

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import SeaTableTypeFilter from './SeaTableTypeFilter';
3838
import { seaServerInfoValue } from './SeaServerInfo';
3939
import { buildSeaPositionalParams, buildSeaNamedParams } from './SeaPositionalParams';
4040
import ParameterError from '../errors/ParameterError';
41+
import { serializeQueryTags } from '../utils';
4142

4243
export interface SeaSessionBackendOptions {
4344
/** The opaque napi `Connection` handle returned by `openSession`. */
@@ -115,19 +116,17 @@ export default class SeaSessionBackend implements ISessionBackend {
115116
/**
116117
* Execute a SQL statement through the napi binding.
117118
*
118-
* Catalog / schema / sessionConf were applied at session open, so
119-
* there are no per-statement options to thread through.
119+
* Catalog / schema / sessionConf were applied at session open. The
120+
* per-statement options threaded here mirror the Thrift backend:
121+
* `ordinalParameters` / `namedParameters` (bound params), `queryTimeout`
122+
* (server wait timeout), `queryTags` (serialised into the conf overlay's
123+
* `query_tags` key), `statementConf` (arbitrary conf overlay), and
124+
* `rowLimit` (SEA-only server-side row cap).
120125
*
121-
* M0 intentionally rejects `queryTimeout`, `namedParameters`, and
122-
* `ordinalParameters` with explicit deferred-to-M1 errors. `useCloudFetch`
123-
* is a no-op on the SEA path — the kernel hardcodes the SEA
124-
* `disposition` to `INLINE_OR_EXTERNAL_LINKS`, and per-statement
125-
* conf overrides have no reader on the kernel; cloud-fetch behaviour
126-
* is governed entirely by the kernel's `ResultConfig` (M1 binding
127-
* surface).
128-
*
129-
* The Thrift backend remains the path for consumers that need any
130-
* of those today.
126+
* `useCloudFetch` is a no-op on the SEA path — the kernel hardcodes the
127+
* SEA `disposition` to `INLINE_OR_EXTERNAL_LINKS`; cloud-fetch behaviour
128+
* is governed by the kernel's `ResultConfig`. `maxRows` is the
129+
* client-side per-fetch chunk size, applied by the facade, not here.
131130
*/
132131
public async executeStatement(statement: string, options: ExecuteStatementOptions): Promise<IOperationBackend> {
133132
this.failIfClosed();
@@ -155,6 +154,27 @@ export default class SeaSessionBackend implements ISessionBackend {
155154
if (options.queryTimeout !== undefined) {
156155
nativeOptions.queryTimeoutSecs = Number(options.queryTimeout);
157156
}
157+
// Server-side row cap (SEA `row_limit`). SEA-only — the Thrift backend has
158+
// no execute-time server cap, so there is no parity obligation here.
159+
if (options.rowLimit !== undefined) {
160+
nativeOptions.rowLimit = Number(options.rowLimit);
161+
}
162+
// Per-statement conf overlay (`statement_conf`) plus query tags. Tags are
163+
// serialised JS-side into the `query_tags` key (the same wire shape the
164+
// Thrift backend produces via `serializeQueryTags` → `confOverlay`), rather
165+
// than via the napi `queryTags` field: napi's `HashMap<String,String>`
166+
// can't represent a null-valued tag, and the kernel rejects setting both
167+
// the `queryTags` field and a `query_tags` conf key.
168+
const serializedQueryTags = serializeQueryTags(options.queryTags);
169+
if (options.statementConf !== undefined || serializedQueryTags !== undefined) {
170+
const statementConf: Record<string, string> = { ...(options.statementConf ?? {}) };
171+
if (serializedQueryTags !== undefined) {
172+
statementConf.query_tags = serializedQueryTags;
173+
}
174+
if (Object.keys(statementConf).length > 0) {
175+
nativeOptions.statementConf = statementConf;
176+
}
177+
}
158178
const hasOptions = Object.keys(nativeOptions).length > 0;
159179

160180
// Submit asynchronously (kernel `wait_timeout=0s`): the server

tests/unit/sea/execution.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,50 @@ describe('SeaSessionBackend', () => {
455455
expect((thrown as Error).message).to.match(/both ordinal and named/);
456456
});
457457

458+
it('executeStatement forwards rowLimit as napi rowLimit', async () => {
459+
const connection = new FakeNativeConnection();
460+
const session = makeSession(connection);
461+
await session.executeStatement('SELECT 1', { rowLimit: 500 });
462+
expect(connection.lastOptions?.rowLimit).to.equal(500);
463+
});
464+
465+
it('executeStatement forwards statementConf verbatim as napi statementConf', async () => {
466+
const connection = new FakeNativeConnection();
467+
const session = makeSession(connection);
468+
await session.executeStatement('SELECT 1', { statementConf: { 'spark.sql.ansi.enabled': 'true' } });
469+
expect(connection.lastOptions?.statementConf).to.deep.equal({ 'spark.sql.ansi.enabled': 'true' });
470+
});
471+
472+
it('executeStatement serialises queryTags into statementConf.query_tags (Thrift wire shape)', async () => {
473+
const connection = new FakeNativeConnection();
474+
const session = makeSession(connection);
475+
await session.executeStatement('SELECT 1', { queryTags: { team: 'data', env: 'prod' } });
476+
expect(connection.lastOptions?.statementConf?.query_tags).to.equal('team:data,env:prod');
477+
// queryTags must NOT use the napi `queryTags` field (can't carry null tags;
478+
// kernel rejects both field + conf key).
479+
expect(connection.lastOptions?.queryTags).to.equal(undefined);
480+
});
481+
482+
it('executeStatement carries a null-valued queryTag as a key-only segment', async () => {
483+
const connection = new FakeNativeConnection();
484+
const session = makeSession(connection);
485+
await session.executeStatement('SELECT 1', { queryTags: { audited: null } });
486+
expect(connection.lastOptions?.statementConf?.query_tags).to.equal('audited');
487+
});
488+
489+
it('executeStatement merges queryTags into a provided statementConf', async () => {
490+
const connection = new FakeNativeConnection();
491+
const session = makeSession(connection);
492+
await session.executeStatement('SELECT 1', {
493+
statementConf: { 'spark.sql.ansi.enabled': 'true' },
494+
queryTags: { team: 'data' },
495+
});
496+
expect(connection.lastOptions?.statementConf).to.deep.equal({
497+
'spark.sql.ansi.enabled': 'true',
498+
query_tags: 'team:data',
499+
});
500+
});
501+
458502
it('executeStatement uses the no-options fast path when nothing is bound', async () => {
459503
const connection = new FakeNativeConnection();
460504
const session = makeSession(connection);

tests/unit/sea/positionalParams.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,25 @@ describe('SeaPositionalParams.buildSeaPositionalParams', () => {
5454
{ sqlType: 'TIMESTAMP', value: '2024-01-15 10:30:00' },
5555
]);
5656
});
57+
58+
it('honours explicit TIMESTAMP_NTZ / TIMESTAMP_LTZ types (kernel param codec)', () => {
59+
expect(
60+
buildSeaPositionalParams([
61+
new DBSQLParameter({ type: DBSQLParameterType.TIMESTAMP_NTZ, value: '2024-01-15 10:30:00' }),
62+
new DBSQLParameter({ type: DBSQLParameterType.TIMESTAMP_LTZ, value: '2024-01-15 10:30:00' }),
63+
]),
64+
).to.deep.equal([
65+
{ sqlType: 'TIMESTAMP_NTZ', value: '2024-01-15 10:30:00' },
66+
{ sqlType: 'TIMESTAMP_LTZ', value: '2024-01-15 10:30:00' },
67+
]);
68+
});
69+
70+
it('routes a Date with explicit TIMESTAMP_NTZ type as NTZ (not the default TIMESTAMP)', () => {
71+
const d = new Date('2024-01-15T10:30:00.000Z');
72+
expect(
73+
buildSeaPositionalParams([new DBSQLParameter({ type: DBSQLParameterType.TIMESTAMP_NTZ, value: d })]),
74+
).to.deep.equal([{ sqlType: 'TIMESTAMP_NTZ', value: d.toISOString() }]);
75+
});
5776
});
5877

5978
describe('SeaPositionalParams.buildSeaNamedParams', () => {

0 commit comments

Comments
 (0)