chore: upgrade SQL kernel to 1.1.0 - #528
vuanhphung wants to merge 1 commit into
Conversation
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
There was a problem hiding this comment.
Verdict: 1 Medium · 1 Low
Straightforward kernel 1.0.0→1.1.0 bump; the mechanical version-string/lockfile/integrity changes are internally consistent. One medium concern: the new JSDoc documents that the kernel now replaces rather than appends the base User-Agent and requires the leading prefix be SEA-allow-listed — the driver sends a NodejsDatabricksSqlConnector/... prefix, so this should be verified against a live warehouse (and the now-stale append-semantics comments in KernelAuth.ts updated). A low note flags the new empty-catalog=matches-nothing semantics for listSchemas/listColumns.
| * token the SEA server keys on while still surfacing the caller's | ||
| * identity. The NodeJS driver folds its `userAgentEntry` into a | ||
| * - `User-Agent` **replaces** the `DatabricksSQLKernel/...` base. | ||
| * Its leading prefix must therefore be allow-listed by the SEA |
There was a problem hiding this comment.
🟡 Medium — This 1.1.0 bump documents a behavior change in kernel UA handling, not just a version string. The old contract was that the kernel appends the caller's User-Agent to its base UA, preserving the DatabricksJDBCDriverOSS/... / DatabricksSQLKernel/... token "the SEA server keys on". The new contract (this diff) is that the caller's User-Agent replaces the kernel base entirely, and "Its leading prefix must therefore be allow-listed by the SEA server."
The driver unconditionally pushes its own composed UA as the last User-Agent entry (lib/kernel/KernelAuth.ts:503-505), and that value starts with NodejsDatabricksSqlConnector/... (buildUserAgentString), not with any Databricks* kernel prefix. Under append semantics the kernel base token survived; under the new replace semantics the effective UA sent to SEA is now NodejsDatabricksSqlConnector/... with no kernel/JDBC prefix.
Please confirm that NodejsDatabricksSqlConnector is on the SEA server's UA allow-list. If SEA gates on the leading prefix (as this JSDoc states), kernel-path connections could be rejected server-side — something the mocked unit suite (1,312 passed) would not catch. If it is allow-listed, no code change is needed, but this should be verified against a live warehouse before release.
Relatedly, the driver's own explanatory comments now contradict the shipped kernel: lib/kernel/KernelAuth.ts:218 ("appends a User-Agent entry to its base UA rather than replacing it") and :454 ("the kernel folds the last User-Agent into its base") describe the pre-1.1.0 append behavior and are now stale. Worth updating in this PR so future readers don't rely on the wrong contract.
| listTables(catalog?: string | undefined | null, schemaPattern?: string | undefined | null, tablePattern?: string | undefined | null, tableTypes?: Array<string> | undefined | null): Promise<Statement> | ||
| /** | ||
| * Columns of tables matching the filter. | ||
| * Columns of tables matching the filter. An empty catalog matches |
There was a problem hiding this comment.
🔵 Low — The 1.1.0 JSDoc for listSchemas / listColumns now specifies that an empty catalog matches nothing. The driver forwards request.catalogName verbatim into listSchemas(request.catalogName, ...) and listColumns(request.catalogName, ...) (KernelSessionBackend.ts:365,383) with no normalization. A caller that passes an empty-string catalogName (as opposed to undefined) expecting a wildcard/all-catalogs result would now silently get zero rows on the kernel path.
Unlike getPrimaryKeys (which explicitly rejects ''/undefined at KernelSessionBackend.ts:401-407), getSchemas/getColumns neither reject nor document this. This is likely benign if callers only ever pass undefined for "all", but the empty-string edge case is now a behavior difference from the Thrift backend and is untested. Consider a unit test asserting the empty-catalog behavior, or normalizing ''→undefined if wildcard semantics are intended.
Summary
Upgrade all eight optional kernel native packages to 1.1.0, pin source builds to the matching kernel release commit, and refresh the generated binding surface. The kernel dependency is now stable rather than experimental, as noted in the changelog.
Test plan
npm run buildenv -u CODEX_CI npm test(1,312 passed)