Skip to content

Commit bafd8b9

Browse files
committed
refactor(sea)!: move catalog/schema/sessionConf from per-statement forwarding to openSession
The napi binding moved initialCatalog/initialSchema/sessionConfig from ExecuteOptions onto ConnectionOptions (matching pyo3) because the kernel does not read StatementSpec::statement_conf — they were silently no-op'd in the per-statement path. Adapter follows. - SeaAuth.ts: extend SeaNativeConnectionOptions with optional catalog / schema / sessionConf (intersection with each auth-mode variant). New SeaSessionDefaults interface for the shared shape. - SeaBackend.ts::openSession: fold OpenSessionRequest.initialCatalog / initialSchema / configuration into the napi options before the openSession call. Drop the SeaSessionBackend.defaults forwarding. - SeaNativeLoader.ts: drop SeaExecuteOptions; executeStatement now takes only the SQL. - SeaSessionBackend.ts: drop SeaSessionDefaults and defaults field; drop per-statement overlay logic. useCloudFetch becomes a no-op on the SEA path (kernel hardcodes disposition to INLINE_OR_EXTERNAL_LINKS; ResultConfig exposure is M1 work). - tests: replace per-statement-forwarding assertions with openSession-arg assertions. 23/23 SEA execution tests pass (143/143 across the SEA suite). Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
1 parent 13e1fce commit bafd8b9

6 files changed

Lines changed: 140 additions & 186 deletions

File tree

lib/sea/SeaAuth.ts

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -49,26 +49,47 @@ const U2M_DEFAULT_REDIRECT_PORT = 8030;
4949
* incompatible with `isolatedModules` and a runtime-coupling hazard.
5050
* The Rust source of truth lives at `native/sea/src/database.rs`.
5151
*/
52-
export type SeaNativeConnectionOptions =
53-
| {
54-
hostName: string;
55-
httpPath: string;
56-
authMode: 'Pat';
57-
token: string;
58-
}
59-
| {
60-
hostName: string;
61-
httpPath: string;
62-
authMode: 'OAuthM2m';
63-
oauthClientId: string;
64-
oauthClientSecret: string;
65-
}
66-
| {
67-
hostName: string;
68-
httpPath: string;
69-
authMode: 'OAuthU2m';
70-
oauthRedirectPort: number;
71-
};
52+
/**
53+
* Session-level defaults shared across all auth-mode variants.
54+
*
55+
* Mirrors `ConnectionOptions.catalog` / `.schema` / `.sessionConf` on
56+
* the napi binding (kernel `Session::builder().defaults(DefaultOpts)`
57+
* and `.session_conf(HashMap)` — the routes that actually populate SEA
58+
* `CreateSession.catalog` / `.schema` / `.session_confs`).
59+
*
60+
* Per-statement overrides do not exist on the kernel surface; both
61+
* pyo3 and napi expose catalog / schema / sessionConf only at session
62+
* creation. Mirror that here so the adapter doesn't promise a
63+
* capability the binding can't honour.
64+
*/
65+
export interface SeaSessionDefaults {
66+
catalog?: string;
67+
schema?: string;
68+
sessionConf?: Record<string, string>;
69+
}
70+
71+
export type SeaNativeConnectionOptions = SeaSessionDefaults &
72+
(
73+
| {
74+
hostName: string;
75+
httpPath: string;
76+
authMode: 'Pat';
77+
token: string;
78+
}
79+
| {
80+
hostName: string;
81+
httpPath: string;
82+
authMode: 'OAuthM2m';
83+
oauthClientId: string;
84+
oauthClientSecret: string;
85+
}
86+
| {
87+
hostName: string;
88+
httpPath: string;
89+
authMode: 'OAuthU2m';
90+
oauthRedirectPort: number;
91+
}
92+
);
7293

7394
function prependSlash(str: string): string {
7495
if (str.length > 0 && str.charAt(0) !== '/') {

lib/sea/SeaBackend.ts

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -89,28 +89,36 @@ export default class SeaBackend implements IBackend {
8989
throw new HiveDriverError('SeaBackend: not connected. Call connect() first.');
9090
}
9191

92+
// Fold session-level defaults from the OpenSessionRequest into the
93+
// napi `ConnectionOptions`. The kernel routes these through
94+
// `Session::builder().defaults(DefaultOpts)` + `.session_conf(...)`
95+
// so they land on the SEA `CreateSession` wire fields, not on each
96+
// per-statement request. Matches pyo3's `Session.__new__` shape.
97+
//
98+
// Only set the optional keys when present so the napi call shape
99+
// stays minimal — keeps wire snapshots / test assertions stable
100+
// for callers who pass no defaults.
101+
const sessionOptions: SeaNativeConnectionOptions = { ...this.nativeOptions };
102+
if (request.initialCatalog !== undefined) {
103+
sessionOptions.catalog = request.initialCatalog;
104+
}
105+
if (request.initialSchema !== undefined) {
106+
sessionOptions.schema = request.initialSchema;
107+
}
108+
if (request.configuration !== undefined) {
109+
sessionOptions.sessionConf = { ...request.configuration };
110+
}
111+
92112
let nativeConnection: SeaNativeConnection;
93113
try {
94-
nativeConnection = (await this.binding.openSession(this.nativeOptions)) as SeaNativeConnection;
114+
nativeConnection = (await this.binding.openSession(sessionOptions)) as SeaNativeConnection;
95115
} catch (err) {
96116
throw decodeNapiKernelError(err);
97117
}
98118

99-
// Merge `request.configuration` (the existing public field for Spark
100-
// conf) with any backend-specific session config. The SEA wire
101-
// protocol applies these per-statement, but we capture them at
102-
// session-open time and forward with every executeStatement to
103-
// preserve session-config semantics.
104-
const sessionConfig = request.configuration ? { ...request.configuration } : undefined;
105-
106119
return new SeaSessionBackend({
107120
connection: nativeConnection!,
108121
context: this.context,
109-
defaults: {
110-
initialCatalog: request.initialCatalog,
111-
initialSchema: request.initialSchema,
112-
sessionConfig,
113-
},
114122
});
115123
}
116124

lib/sea/SeaNativeLoader.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,19 @@
2626
import type {
2727
Connection as NativeConnection,
2828
Statement as NativeStatement,
29-
ExecuteOptions,
3029
ArrowBatch,
3130
ArrowSchema,
3231
} from '@sea-native';
3332
import type { SeaNativeConnectionOptions } from './SeaAuth';
3433

35-
export type { ExecuteOptions, ArrowBatch, ArrowSchema, SeaNativeConnectionOptions };
34+
export type { ArrowBatch, ArrowSchema, SeaNativeConnectionOptions };
3635
export type Connection = NativeConnection;
3736
export type Statement = NativeStatement;
3837

3938
// Back-compat aliases for consumers that landed before the path-alias
4039
// refactor renamed these. New code should import the un-prefixed names.
4140
export type SeaNativeConnection = NativeConnection;
4241
export type SeaNativeStatement = NativeStatement;
43-
export type SeaExecuteOptions = ExecuteOptions;
4442
export type SeaArrowBatch = ArrowBatch;
4543
export type SeaArrowSchema = ArrowSchema;
4644

lib/sea/SeaSessionBackend.ts

Lines changed: 23 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -31,33 +31,14 @@ import {
3131
import Status from '../dto/Status';
3232
import InfoValue from '../dto/InfoValue';
3333
import HiveDriverError from '../errors/HiveDriverError';
34-
import { SeaNativeConnection, SeaExecuteOptions } from './SeaNativeLoader';
34+
import { SeaNativeConnection } from './SeaNativeLoader';
3535
import { decodeNapiKernelError } from './SeaErrorMapping';
3636
import SeaOperationBackend from './SeaOperationBackend';
3737

38-
/**
39-
* Per-session defaults that apply to every `executeStatement` issued
40-
* through this backend. Captured at `SeaBackend.openSession()` time from
41-
* the `OpenSessionRequest` — `initialCatalog` / `initialSchema` /
42-
* `sessionConfig`.
43-
*
44-
* The napi binding routes these to the kernel's `statement_conf` map,
45-
* which the SEA wire treats as session-scoped parameters. They are
46-
* forwarded with every `executeStatement` call so the JDBC-style
47-
* "session config" semantics are preserved even though SEA's wire
48-
* protocol is statement-scoped.
49-
*/
50-
export interface SeaSessionDefaults {
51-
initialCatalog?: string;
52-
initialSchema?: string;
53-
sessionConfig?: Record<string, string>;
54-
}
55-
5638
export interface SeaSessionBackendOptions {
5739
/** The opaque napi `Connection` handle returned by `openSession`. */
5840
connection: SeaNativeConnection;
5941
context: IClientContext;
60-
defaults?: SeaSessionDefaults;
6142
/** Optional override for `id`. Defaults to a fresh UUIDv4. */
6243
id?: string;
6344
}
@@ -72,30 +53,26 @@ export interface SeaSessionBackendOptions {
7253
* backend continues to handle the metadata path by default (callers
7354
* opt into SEA via `ConnectionOptions.useSEA`).
7455
*
75-
* **Session config flow:** the SEA wire protocol is statement-scoped,
76-
* so "session config" semantics (Spark conf, `initialCatalog`,
77-
* `initialSchema`) are emulated by forwarding the same defaults with
78-
* every `executeStatement` call. Per-statement overrides on
79-
* `ExecuteStatementOptions` are reserved for M1; M0 carries only the
80-
* defaults captured at session-open time plus the `useCloudFetch`
81-
* boolean projected onto `sessionConfig.use_cloud_fetch` for the
82-
* kernel.
56+
* **Session config flow:** catalog / schema / sessionConf are applied
57+
* once at session creation (kernel `Session::builder().defaults()` +
58+
* `.session_conf()` → SEA `CreateSession.catalog` / `.schema` /
59+
* `.session_confs`) and remain in effect for every statement run on
60+
* the resulting napi `Connection`. No per-statement forwarding is
61+
* needed — that pattern was removed when the napi binding moved these
62+
* onto `openSession` to match pyo3.
8363
*/
8464
export default class SeaSessionBackend implements ISessionBackend {
8565
private readonly connection: SeaNativeConnection;
8666

8767
private readonly context: IClientContext;
8868

89-
private readonly defaults: SeaSessionDefaults;
90-
9169
private readonly _id: string;
9270

9371
private closed = false;
9472

95-
constructor({ connection, context, defaults, id }: SeaSessionBackendOptions) {
73+
constructor({ connection, context, id }: SeaSessionBackendOptions) {
9674
this.connection = connection;
9775
this.context = context;
98-
this.defaults = defaults ?? {};
9976
this._id = id ?? uuidv4();
10077
}
10178

@@ -108,13 +85,21 @@ export default class SeaSessionBackend implements ISessionBackend {
10885
}
10986

11087
/**
111-
* Execute a SQL statement through the napi binding. Merges the
112-
* session-level defaults (`initialCatalog` / `initialSchema` /
113-
* `sessionConfig`) with the per-call `useCloudFetch` override.
88+
* Execute a SQL statement through the napi binding.
89+
*
90+
* Catalog / schema / sessionConf were applied at session open, so
91+
* there are no per-statement options to thread through.
11492
*
11593
* M0 intentionally rejects `queryTimeout`, `namedParameters`, and
116-
* `ordinalParameters` with explicit deferred-to-M1 errors. The Thrift
117-
* backend remains the path for consumers that need any of those today.
94+
* `ordinalParameters` with explicit deferred-to-M1 errors. `useCloudFetch`
95+
* is a no-op on the SEA path — the kernel hardcodes the SEA
96+
* `disposition` to `INLINE_OR_EXTERNAL_LINKS`, and per-statement
97+
* conf overrides have no reader on the kernel; cloud-fetch behaviour
98+
* is governed entirely by the kernel's `ResultConfig` (M1 binding
99+
* surface).
100+
*
101+
* The Thrift backend remains the path for consumers that need any
102+
* of those today.
118103
*/
119104
public async executeStatement(statement: string, options: ExecuteStatementOptions): Promise<IOperationBackend> {
120105
this.failIfClosed();
@@ -131,24 +116,9 @@ export default class SeaSessionBackend implements ISessionBackend {
131116
);
132117
}
133118

134-
// Merge session-level sessionConfig with per-statement useCloudFetch.
135-
// The kernel accepts only string-valued conf values; booleans are
136-
// String()'d to "true"/"false" matching the existing Thrift conf
137-
// convention.
138-
const sessionConfig: Record<string, string> = { ...(this.defaults.sessionConfig ?? {}) };
139-
if (options.useCloudFetch !== undefined) {
140-
sessionConfig.use_cloud_fetch = String(options.useCloudFetch);
141-
}
142-
143-
const executeOptions: SeaExecuteOptions = {
144-
initialCatalog: this.defaults.initialCatalog,
145-
initialSchema: this.defaults.initialSchema,
146-
sessionConfig: Object.keys(sessionConfig).length > 0 ? sessionConfig : undefined,
147-
};
148-
149119
let nativeStatement;
150120
try {
151-
nativeStatement = await this.connection.executeStatement(statement, executeOptions);
121+
nativeStatement = await this.connection.executeStatement(statement);
152122
} catch (err) {
153123
throw decodeNapiKernelError(err);
154124
}

native/sea/index.d.ts

Lines changed: 36 additions & 68 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)