feat(kernel): consolidated DAIS gap-closure + PuPr feature set by mani-mathur-arch · Pull Request #399 · databricks/databricks-sql-go

mani-mathur-arch · 2026-07-14T10:51:20Z

What

Extends the SEA-via-kernel backend for the Go driver with a set of authentication, session-setup, type-rendering, query-execution, and telemetry features. 24 files, +1265 / −161.

Features

Authentication

OAuth M2M / U2M — the kernel drives its own auth flow from cfg.Authenticator. Adds a kernel.Auth value descriptor + resolveKernelAuth, wired through the set_auth_pat / _m2m / _u2m C-ABI setters.

Session setup

Initial namespace — post-connect USE CATALOG / USE SCHEMA, with identifier quoting (quoteIdent).
Metric-view metadata — config.EffectiveSessionParams() folds the server conf in a backend-neutral way.

Type rendering

INTERVAL day-time & year-month rendering in internal/arrowscan.
Parity coverage for TIMESTAMP vs TIMESTAMP_NTZ and VARIANT / GEOMETRY (live-probed against both backends: NTZ shifts on both; variant/geometry arrive as strings).

Query execution & telemetry

Query parameter binding via the kernel raw-param C ABI (kernel_statement_bind_parameter).
Kernel errors surfaced as DBExecutionError carrying both the sqlstate and the server query id (empty-queryId → ctx fallback), so QueryId() is populated on the kernel error path.
Server query id exposed via a real StatementID() accessor (kernel_executed_statement_query_id) and threaded into EXECUTE_STATEMENT telemetry.

Notable behavior

Bound query parameters are now supported (previously rejected at connect); they are bound in execute via the kernel raw-param C ABI.
Staging operations remain rejected — the kernel path can't perform the required local file transfer and the C ABI surfaces no staging signal.
doc.go updated to describe OAuth / namespace / metric-view / params as supported and staging as unsupported; the stale INTERVAL caveat is dropped.

Testing

Default pure-Go (CGO_ENABLED=0): go build, go vet, go test ./... — 24 packages ok, 0 fail.
Tagged kernel path (CGO_ENABLED=1 -tags databricks_kernel, linked against a locally-built kernel .a at KERNEL_REV): go build, go vet, go test ./... — 24 packages ok, 0 fail.
gofmt clean across all 24 changed files.
Env-gated e2e tests (kernel_e2e_test.go) verified via a temporary trigger on a pr in the stack

Merge gate

Re-pin KERNEL_REV at merge time. It currently points at the kernel dependency's PR-head SHA, which is GC-able once that kernel change merges; bump it to the resulting kernel main SHA (bare token — the Makefile does $(shell cat KERNEL_REV)), re-sync the cgo drift assertions in cgo.go if any signatures changed, and run make test-kernel against the new rev.

This pull request and its description were written by Isaac.

…initial namespace Close the four DAIS-scope rows the SEA-via-kernel backend previously rejected at connect time. All are Go-side only (no new kernel dependency): the OAuth setters are on merged kernel #162, metric-view is an existing session conf, and the namespace uses plain SQL. - Metric view: config.EffectiveSessionParams() derives the server conf (spark.sql.thriftserver.metadata.metricview.enabled) once, backend-neutrally, so both backends send the identical conf. The Thrift OpenSession special-case is removed (behaviour-preserving); the kernel forwards it via SessionConf. Reject dropped; reclassified forwarded. - Initial namespace: applied post-connect via USE CATALOG / USE SCHEMA (the OSS ODBC workaround) since the kernel C ABI has no catalog/schema setter. quoteIdent (untagged) backtick-quotes identifiers; a USE failure fails connect and closes the session. Reject dropped; reclassified forwarded. - OAuth M2M/U2M: the kernel drives its own OAuth flow from raw credentials (mirroring pyo3/napi and the Node/Python kernel bindings), read off cfg.Authenticator — the single source of truth (last-writer-wins, matching Thrift). The m2m/u2m authenticators expose auth.M2MCredentialsProvider / auth.U2MCredentialsProvider; resolveKernelAuth type-switches them and returns a *kernelAuth descriptor. KernelBackend.setAuth branches to set_auth_pat / set_auth_m2m / set_auth_u2m; U2M uses Go's cloud-inferred client id (kernel defaults for scopes/port). No new config fields. Verified: default CGO_ENABLED=0 suite + golangci-lint v2.12.2 clean; tagged databricks_kernel unit tests (auth-mode -> setter mapping, quoteIdent); live staging e2e for initial namespace (current_catalog/current_schema) and metric-view (session opens + queries; the conf is not SET-introspectable on either backend). M2M/U2M covered by unit tests (no staging service principal; U2M is interactive). Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

… single-source auth, coverage + docs Remediation of the code-review pass on the DAIS gap-closure work. Verified against source; full default + tagged suites, golangci-lint v2.12.2, and live staging e2e all green. - Move the OAuth credential-provider interfaces (M2MCredentialsProvider / U2MCredentialsProvider) out of the public auth package into internal/backend/kernel, so the secret-reading capability is not part of the driver's public API. The unexported m2m/u2m authenticators satisfy them structurally. - Collapse the duplicate auth descriptor: validateKernelConfig/resolveKernelAuth now return kernel.Auth directly (its type is in an untagged file, so the default build builds it cgo-free); dropped dbsql.kernelAuth, kernelAuthMode, and toKernelAuth (which also removed a stale build-tag comment). - Route the initial-namespace failure-path session close through call() so a failed close is logged (via lastError's Warn), mirroring CloseSession. - Add an env-guarded live M2M e2e (TestKernelE2EM2M, skips without DATABRICKS_CLIENT_ID/_SECRET) and a last-writer-wins auth regression test; the resolveKernelAuth -> kernel.Auth path is table-tested for M2M/U2M. - Document: U2M is interactive (browser on cache-miss, blocks up to the kernel's ~120s callback timeout, connect ctx deadline not honored during that window, no C-ABI override — use PAT/M2M for headless); the U2M Scopes/RedirectPort fields are dormant-but-wired (no Go option feeds them yet); the metric-view e2e is a deliberate connect-smoke (routing asserted in TestEffectiveSessionParams). Custom M2M scopes remain unforwardable over the C ABI (no scopes arg on set_auth_m2m) — a kernel gap shared with ODBC, no authz impact (all-apis always requested); tracked in the kernel-gaps notes rather than worked around. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

The kernel backend returns INTERVAL columns as native arrow duration (day-time) and month-interval (year-month) values, whereas the Thrift path receives them pre-formatted from the server (its native-interval config is off in prod, so it never scans a duration/month-interval array). Format them Go-side in the shared untagged arrowscan package to the same strings the Thrift path returns — "D HH:MM:SS.nnnnnnnnn" and "years-months", negatives signed — so a query's result is identical across backends. Replaces the fail-loud "intervals are not yet handled" default arm with the two type arms; golden-string unit tests (day/day-to-sec/seconds-unit/negative, year/year-month/months/negative) run in the default CGO_ENABLED=0 build. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

Over Arrow the kernel delivers TIMESTAMP with a tz ("UTC") and TIMESTAMP_NTZ with an empty tz, but — like the Thrift path — the driver ignores that field and renders both via ToTime + .In(loc). The LTZ-vs-NTZ difference is carried entirely by the instant the server sends, not by the client inspecting the tz, so no arrowscan change is needed: the existing code already matches Thrift. Verified live on both backends (America/New_York + Asia/Kolkata, including a DST spring-forward literal, and nested/null shapes): kernel == Thrift byte-for-byte for both types. Add an untagged parity case (TimeZone "UTC" vs "") so a future "don't shift NTZ" change — which looks correct in isolation but would diverge from Thrift, which shifts NTZ too — fails default CI, plus a live e2e pinning the round-trip. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

VARIANT and GEOMETRY need no special rendering on the kernel path: verified live on both backends, both arrive over Arrow as plain STRING columns — a top-level VARIANT is its JSON text ({"a":1,"b":[2,3]}), a scalar VARIANT is "42", and GEOMETRY is its WKT "POINT(1 2)". Nested inside a container the variant/geometry element is a string leaf, rendered as a quoted, JSON-escaped string (the variant's own JSON is escaped as text, NOT re-parsed) — identical on both backends. Add untagged parity cases: a top-level string equivalence (variant object / scalar / geometry WKT) and nested string-leaf cases in an array, so the string arm's handling of these types can't silently drift between backends. GEOGRAPHY is intentionally excluded — not enabled on the benchmark warehouse and no consumer has asked for it. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

A failed kernel query returned the raw *KernelError, so consumers doing errors.As(err, &DBExecutionError) — the way they inspect Thrift failures — didn't get SqlState()/QueryId()/IsRetryable() through the standard interface. kernelOp.ExecutionError now digs the sqlstate out of the underlying *KernelError and wraps the cause via NewExecutionErrorWithState, so kernel query failures surface with the same DBExecutionError shape as Thrift. Adds the neutral NewExecutionErrorWithState to the untagged internal/errors package (Thrift's NewExecutionError needs a TGetOperationStatusResp the kernel backend can't produce), unit-tested in the default CGO_ENABLED=0 build. Parity is type + SQLSTATE, not byte-identical text — kernel messages are richer (they carry the SQL error class + suggestions). Verified live: unknown table → 42P01, unknown column → 42703, byte-identical sqlstate to Thrift. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

…al caveat Record what the kernel backend inherits unchanged above the backend seam — the database/sql connection pool (each conn wraps one kernel session), per-connection CREATE_SESSION / DELETE_SESSION telemetry (recorded unconditionally in connector.go, backend-agnostic), and the telemetry exporter's circuit breaker — and that result types render byte-for-byte with Thrift (scalars, exact DECIMAL, TIMESTAMP / TIMESTAMP_NTZ, INTERVAL, nested + VARIANT as JSON, GEOMETRY as WKT). Remove the now-stale "INTERVAL types are not yet handled by the kernel scanner" caveat (intervals render now), and narrow the telemetry caveat to what is actually missing: only EXECUTE_STATEMENT telemetry (gated on a per-statement query id the kernel C ABI doesn't yet surface) — CREATE_SESSION / DELETE_SESSION are unaffected. Add a live-verified connection-pool e2e (40 concurrent queries over pool cap 8) backing the inherited-pool claim. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

The kernel backend rejected bound parameters at execute time. Now it binds them: the driver's backend.Param{Name, Type, Value} maps 1:1 onto the kernel's kernel_statement_bind_parameter (K1) — value already stringified, Type the Databricks SQL type name, empty Name → positional, nil Value → SQL NULL ("VOID"). bindParams runs after set_sql (which clears any prior binds), using the existing newCStr/newCStrOrNull helpers and the call() FFI-safety wrapper; a bind failure closes the statement and surfaces via toStatementError. Removes the fail-loud reject in Execute (and its now-unused errors import). The old TestExecuteRejectsParams is repurposed as TestExecuteHandleLessOpContract (the non-nil handle-less Operation contract, now driven by a nil-session failure since params no longer reject). Live parity: 10 cases (positional/named, each scalar type, NULL, multi-param, predicate) produce byte-identical output on the kernel and Thrift backends. Requires a kernel build carrying kernel_statement_bind_parameter; the KERNEL_REV pin is bumped to the K1 merge SHA when it lands. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

kernelOp.StatementID() returned "", so the kernel backend emitted no EXECUTE_STATEMENT telemetry and QueryIdCallback fired with an empty id (connection.go gates both on a non-empty statement id). Wire StatementID() to the server query id via kernel_executed_statement_query_id (K1), captured at execute time into a cached field — the same lifetime discipline as affectedRows, since the C accessor returns a pointer borrowed from the exec handle and the op is closed (nulling exec) before StatementID() is read on some paths. C.GoString deep-copies out of the borrowed string. Live e2e: a registered QueryIdCallback fires with a non-empty server id after a kernel query. Updates doc.go — bound parameters (c6) and EXECUTE_STATEMENT telemetry are now supported; the remaining kernel-backend limitation is batch-boundary (not mid-fetch) read cancellation. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

Bumps the kernel pin from the #163 canceller rev to the tip of the PuPr statement-surface branch (databricks-sql-kernel#165), which adds kernel_statement_bind_parameter and kernel_executed_statement_query_id — the two C-ABI symbols the bound-parameter (c6) and EXECUTE_STATEMENT-telemetry (c7) commits link against. Re-pin to the squash-merge SHA once #165 lands. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

mani-mathur-arch · 2026-07-15T08:49:32Z

Two additional review findings from the PR pass could not be anchored inline because GitHub does not expose those exact lines in this PR diff:

Kernel retryability is still dropped from DBExecutionError. KernelError.Retryable is copied from the C ABI, but kernelOp.ExecutionError only forwards sqlstate/query id into NewExecutionErrorWithState; DBError.IsRetryable() is derived from internal/errors.RetryableError in the cause chain. A retryable kernel execution failure will therefore surface as non-retryable unless the cause is wrapped when ke.Retryable is true, with a test asserting DBExecutionError.IsRetryable().
The cancelled execute branch skips session-fatal eviction. If ctx.Err() != nil while kernel_statement_execute returns a session-fatal KernelError, the code returns only ctx.Err() and bypasses k.evictIfSessionFatal(execErr), leaving a dead session potentially valid in the pool and dropping kernel metadata from the returned error. It should still classify/evict on execErr while preserving cancellation semantics.

…ction, bind-mapping test Addresses the review pass on this PR (comments left on #399). Fixes the actionable set; a follow-up Isaac re-review then flagged that one of the requested changes (surfacing the kernel Retryable flag) was itself unsafe, so that one is intentionally NOT made — see below. - Interval negation overflow (High): formatDayTimeInterval / formatYearMonthInterval negated the full magnitude up front (v = -v). At math.MinInt64 (day-time µs) / math.MinInt32 (year-month) that wraps back negative, so every component came out negative AND a '-' was prepended — doubly-negated garbage — and both are representable Spark interval bounds. Now derive each component from the signed value and take its magnitude when formatting (abs64); widen year-month to int64 before negating. Adds MinInt64 (µs + ns) and MinInt32 regression cases. - Cancelled-execute skipped session-fatal eviction (Med): when execErr raced a ctx cancel, execute returned ctx.Err() without calling evictIfSessionFatal, leaving a dead conn marked valid in the pool. Hoist the evict above the ctx-cancelled branch so it fires on both paths (drained watcher first, so no race). Also wrap BOTH the ctx error and the kernel error with two %w verbs so errors.Is(context.DeadlineExceeded) still matches AND the *KernelError (sqlstate/queryId) stays reachable via errors.As instead of being dropped. - Bind mapping had no executing coverage (Med): the live Param-binding proof (TestKernelParamsVsThrift) needs a warehouse and only runs in the credentialed nightly job, so the positional/named + SQL-NULL/empty-string decision shipped untested at PR time. Extract that pure decision into an untagged paramBindArg (bindparams.go), consumed by the cgo bindParams, and unit-test it under CGO_ENABLED=0 (TestParamBindArg). Rename a comment's dead TestKernelE2EParams reference to the real tests. - Stale StatementID() comments (Low): both said StatementID() is "" on this backend, but this PR made it return the real server id on the success path. Scope the empty-id claim to the execute-error path. NOT changed (Isaac re-review, MAJOR): surfacing the kernel's Retryable flag on kernelOp.ExecutionError. That is exclusively the post-submission path, where a network/unavailable failure may have already committed a non-idempotent INSERT/UPDATE/MERGE — reporting IsRetryable()==true would invite an app to double-write, and it diverges from the Thrift path (always non-retryable here). This mirrors toStatementError refusing driver.ErrBadConn for the same reason. sqlState/queryId extraction is unchanged; TestExecutionErrorNeverRetryable pins that a Retryable KernelError still reports IsRetryable()==false. The connect-phase path (toConnError), where the retryable signal IS safe, is unaffected. Verified: default (CGO_ENABLED=0) + kernel-tagged suites pass, gofmt clean, Isaac review clean (0 final comments). Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

mani-mathur-arch · 2026-07-15T11:19:36Z

Both points addressed in f5ea652 — one fixed, one deliberately declined:

1. Retryability — intentionally NOT changed. A follow-up Isaac review flagged wrapping the kernel Retryable flag here as MAJOR: kernelOp.ExecutionError is exclusively the post-submission (execute / result-read) path, where a network/unavailable failure may have already committed a non-idempotent INSERT/UPDATE/MERGE. Surfacing IsRetryable() == true there would invite an app keying retry on it (the doc.go recipe) to double-write, and it diverges from the Thrift path, which always builds a non-retryable execution error here. This mirrors toStatementError refusing driver.ErrBadConn for the same reason. Added TestExecutionErrorNeverRetryable pinning that a Retryable KernelError still reports IsRetryable() == false; sqlState/queryId extraction is unchanged. The connect-phase path (toConnError), where the retryable signal is safe, is unaffected.

2. Cancelled-execute eviction — fixed. Hoisted k.evictIfSessionFatal(execErr) above the ctx.Err() branch so a session-fatal failure racing a cancel still evicts the dead conn from the pool. Also wrapped both the ctx error and the kernel error with two %w verbs, so errors.Is(err, context.DeadlineExceeded/Canceled) still matches and the *KernelError (sqlstate/queryId) stays reachable via errors.As instead of being dropped.

…rnel Follow-up to the review round: the unsupported-authenticator default case in resolveKernelAuth still returned a plain errors.New, while every other unsupported kernel option wraps ErrNotSupportedByKernel and doc.go advertises that errors.Is(err, ErrNotSupportedByKernel) detects any unsupported kernel feature. So this PR shipped a documented contract its own code broke for token-provider / external / federated auth. Wrap it with %w to honor the contract (same fix #403 makes one commit up the stack — matching its wording so the two converge cleanly on rebase). The empty-PAT case stays unwrapped: a missing token is misconfiguration to fix, not a feature the kernel can't honor. Tighten the "non-PAT/non-OAuth authenticator rejected" test to assert errors.Is(err, ErrNotSupportedByKernel) instead of only err != nil, so the contract is pinned rather than documented as an exception. Verified: default-build suite passes, go vet + gofmt clean. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>

This was referenced Jul 14, 2026

feat(kernel): richer TLS Go options — CA bundle + independent hostname skip #400

Open

feat(kernel): route kernel logging through the driver log level + correlate #401

Open

mani-mathur-arch force-pushed the mani/sea-kernel-consolidated branch from 543c7a2 to ba466d3 Compare July 14, 2026 15:36

Base automatically changed from mani/sea-kernel-backend to main July 14, 2026 17:52

mani-mathur-arch force-pushed the mani/sea-kernel-consolidated branch from ba466d3 to f35b387 Compare July 14, 2026 18:42

mani-mathur-arch added 9 commits July 14, 2026 18:47

mani-mathur-arch force-pushed the mani/sea-kernel-consolidated branch from f35b387 to d0c999e Compare July 14, 2026 18:48

mani-mathur-arch force-pushed the mani/sea-kernel-consolidated branch from d0c999e to 2caa354 Compare July 14, 2026 18:53

mani-mathur-arch changed the title ~~feat(kernel): consolidated DAIS gap-closure + PuPr feature set (supersedes #395/#396/#397)~~ feat(kernel): OAuth, initial namespace, metric-view, param binding, extended-type rendering & query-id telemetry for the SEA-via-kernel backend Jul 15, 2026

mani-mathur-arch changed the title ~~feat(kernel): OAuth, initial namespace, metric-view, param binding, extended-type rendering & query-id telemetry for the SEA-via-kernel backend~~ feat(kernel): consolidated DAIS gap-closure + PuPr feature set Jul 15, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(kernel): consolidated DAIS gap-closure + PuPr feature set #399

feat(kernel): consolidated DAIS gap-closure + PuPr feature set #399
mani-mathur-arch wants to merge 12 commits into
mainfrom
mani/sea-kernel-consolidated

mani-mathur-arch commented Jul 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mani-mathur-arch commented Jul 15, 2026

Uh oh!

mani-mathur-arch commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

mani-mathur-arch commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Features

Notable behavior

Testing

Merge gate

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mani-mathur-arch commented Jul 15, 2026

Uh oh!

mani-mathur-arch commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mani-mathur-arch commented Jul 14, 2026 •

edited

Loading