Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion KERNEL_REV
Original file line number Diff line number Diff line change
@@ -1 +1 @@
4d301455f7e70de9cb747e0f1143b8a3df8c5b48
f29957733c8f6d79a7212bc8a50cf4e5ffa8dd69

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] KERNEL_REV pins an unmerged PR-branch head → non-reproducible / breakable CI

f299577… is the head of kernel PR #178 (OPEN). The required symbols (set_retry_config, apply_client_result_overrides) exist only on that branch, not kernel main (verified). A PR head is a mutable ref: a squash-merge / rebase / force-push moves it and GitHub GCs the orphaned commit, after which the kernel-lib fetch of refs/pull/*/head can no longer reach this SHA and every cold (cache-miss) CI build goes red with no source change here.

The PR body already commits to re-pinning to the squash-merge SHA once #178 lands — this should block merge, and ideally #178 merges first so the pin points at a main commit.


Posted by code-review-squad · /full-review · feedback: #code-review-squad-feedback

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this is already called out in the pr description. This will be taken care of while merging.

107 changes: 100 additions & 7 deletions connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,16 @@ func (c *connector) Connect(ctx context.Context) (driver.Conn, error) {
if c.cfg.UseKernel {
be, err = newKernelBackend(ctx, c.cfg)
} else {
// The experimental WithKernel* TLS options have no Thrift-path equivalent —
// reject them loudly rather than silently ignore, so a caller who sets a
// trusted-CA bundle / an independent hostname skip and forgets WithUseKernel
// learns the option had no effect instead of connecting with a
// weaker-than-intended (or unconfigured) TLS trust store.
// The experimental WithKernel* options have no Thrift-path equivalent — reject
// them loudly rather than silently ignore, so a caller who sets one (a
// trusted-CA bundle, a hostname-verify skip, a proxy, a retry budget, or a
// CloudFetch chunk cap) and forgets WithUseKernel learns the option had no
// effect instead of connecting as if it were never set. Every WithKernel*
// option allocates KernelExperimental, so this one gate covers them all; the
// message names the family rather than a stale subset that drifts as options
// are added.
if c.cfg.KernelExperimental != nil {
return nil, fmt.Errorf("databricks: a WithKernel* option "+
"(WithKernelTrustedCerts / WithKernelSkipHostnameVerify) %w; "+
return nil, fmt.Errorf("databricks: a WithKernel* option %w; "+
"add WithUseKernel(true) or remove it", dbsqlerr.ErrRequiresKernelBackend)
}
be, err = thrift.New(ctx, c.cfg, c.client)
Expand Down Expand Up @@ -97,6 +99,15 @@ func (c *connector) Connect(ctx context.Context) (driver.Conn, error) {
if conn.telemetry != nil {
log.Debug().Msg("telemetry initialized for connection")
conn.telemetry.RecordOperation(ctx, conn.id, "", telemetry.OperationTypeCreateSession, sessionLatencyMs, nil)
// Connection-configuration telemetry on the kernel path only, so the
// default (Thrift) path's emitted telemetry stays byte-identical (the
// Thrift path has never populated DriverConnectionParameters). Emits mode /
// auth mech+flow / proxy / arrow / query-tags / metric-view for the
// just-opened session. Gated on the kernel backend, not just WithUseKernel,
// so it never fires when the kernel wasn't actually selected.
if _, ok := be.(*thrift.Backend); !ok {
conn.telemetry.RecordConnectionConfig(ctx, conn.id, kernelConnectionTelemetry(c.cfg))
}
}

// ServerProtocolVersion is Thrift-specific (not on the neutral backend
Expand Down Expand Up @@ -603,3 +614,85 @@ func WithKernelSkipHostnameVerify() ConnOption {
kernelExperimental(c).TLSSkipHostnameVerify = true
}
}

// KernelProxy is the explicit-proxy configuration for WithKernelProxy. Its fields
// are named so a call site can't transpose the credentials — the four values are
// all strings, so a positional signature would let a Username/Password swap (or a
// misplaced BypassHosts) compile cleanly and fail only at runtime with wrong proxy
// credentials.
type KernelProxy struct {
// URL is the proxy URL (e.g. "http://proxy.internal:3128"). Required; empty is a
// no-op, leaving the environment-derived proxy (if any) in effect.
URL string
// Username / Password are optional out-of-band basic-auth credentials, supplied
// here rather than embedded in the URL userinfo. Empty means unset.
Username string
Password string
// BypassHosts is an optional comma-separated no-proxy host list. NO_PROXY is
// consumed during environment resolution and not forwarded to the kernel, so this
// is the only way to give the kernel a structured bypass list. Empty means unset.
BypassHosts string
}

// WithKernelProxy configures an explicit HTTP proxy for the kernel backend, with
// optional out-of-band basic-auth credentials and a comma-separated bypass
// (no-proxy) host list. It overrides the HTTP(S)_PROXY / NO_PROXY environment the
// kernel path otherwise mirrors from the Thrift path.
//
// Use this instead of the proxy environment when you need the "advanced" fields
// the env-var path can't express: a structured bypass list (NO_PROXY is consumed
// during environment resolution, not forwarded to the kernel) or basic-auth
// credentials supplied out of band rather than embedded in the URL userinfo.
// KernelProxy.Username / Password / BypassHosts may be empty (passed to the kernel
// as NULL, i.e. unset). An empty KernelProxy.URL is a no-op — the environment-derived
// proxy, if any, stays in effect. A malformed URL is rejected at connect
// (errors.Is ErrInvalidKernelConfig).
//
// An explicit WithKernelProxy takes precedence over the environment: consulting
// both would be ambiguous, and an explicit proxy is a deliberate override.
//
// EXPERIMENTAL, kernel-only: the default (Thrift) backend rejects this at connect.
func WithKernelProxy(p KernelProxy) ConnOption {
return func(c *config.Config) {
ke := kernelExperimental(c)
ke.ProxyURL = p.URL
ke.ProxyUsername = p.Username
ke.ProxyPassword = p.Password
ke.ProxyBypassHosts = p.BypassHosts
}
}

// WithKernelRetryOverallTimeout sets the cumulative retry budget across all
// attempts on the kernel backend — the total time the kernel may spend retrying a
// single logical request before giving up. This is the 4th retry knob, alongside
// the backoff bounds and max attempts carried by the backend-neutral WithRetries
// (RetryWaitMin / RetryWaitMax / RetryMax, which the kernel path also honors).
//
// It is a kernel-only option because the Thrift-path WithRetries surface has no
// overall-budget equivalent; it mirrors the pyo3/napi retry_overall_timeout knob.
// Zero (the default) keeps the kernel's built-in budget (900s).
//
// EXPERIMENTAL, kernel-only: the default (Thrift) backend rejects this at connect.
func WithKernelRetryOverallTimeout(d time.Duration) ConnOption {
return func(c *config.Config) {
kernelExperimental(c).RetryOverallTimeout = d
}
}

// WithKernelMaxChunksInMemory bounds how many decompressed CloudFetch chunks the
// kernel holds in memory at once on the kernel backend — the knob that trades
// large-result throughput for peak RSS. Lower it (e.g. 4) to cap memory on wide,
// row-heavy result sets; raise it for more download parallelism at higher memory.
// A value <= 0 (the default) leaves the kernel's built-in default (16) in place.
//
// It is forwarded as the kernel's client-only "cloudfetch_max_chunks_in_memory"
// session conf, which the kernel applies to its result config and strips before
// the SEA wire — so it never reaches the server.
//
// EXPERIMENTAL, kernel-only: the default (Thrift) backend has no in-memory-chunk
// knob and rejects this at connect.
func WithKernelMaxChunksInMemory(n int) ConnOption {
return func(c *config.Config) {
kernelExperimental(c).MaxChunksInMemory = n
}
}
56 changes: 44 additions & 12 deletions doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,14 @@ via the authType=oauthU2M DSN param); reading scalar, nested, and complex-typed
results (CloudFetch is transparent); bound query parameters (positional and named);
context cancellation during execute; the initial namespace (WithInitialNamespace,
applied post-connect via USE CATALOG / USE SCHEMA); metric-view metadata
(WithEnableMetricViewMetadata); and the TLS, proxy, and session-conf (query tags,
statement timeout, time zone) options. Nothing is silently ignored: WithTimeout, a
retries-disabling WithRetries, token-provider / external / federated
authenticators, and custom M2M OAuth scopes (the kernel applies its own) are
rejected at connect; staging (PUT/GET/REMOVE on a Unity Catalog volume) is
rejected at execute. WithMaxRows and positive-limit WithRetries are
accepted but inert (the kernel manages fetching and retries below the C ABI).
(WithEnableMetricViewMetadata); the retry / backoff policy (WithRetries:
RetryWaitMin / RetryWaitMax / RetryMax, including the disable form, forwarded to the
kernel's HTTP retry config); and the TLS, proxy, and session-conf (query tags,
statement timeout, time zone) options. Nothing is silently ignored: WithTimeout,
token-provider / external / federated authenticators, and custom M2M OAuth scopes
(the kernel applies its own) are rejected at connect; staging (PUT/GET/REMOVE on a
Unity Catalog volume) is rejected at execute. WithMaxRows is accepted but inert (the
kernel manages fetching below the C ABI).

OAuth U2M is interactive: on a cache miss, connecting launches the system browser and
blocks until login completes or the kernel's ~120s callback timeout expires. Because
Expand All @@ -233,30 +234,61 @@ backend exposes a U2M-scopes option, so this is a fixed difference between the t
default sets, not a dropped setting; both authorize against the built-in public
client.

Experimental kernel-only TLS options (rejected by the default backend; the
Experimental kernel-only options (rejected by the default backend; the
WithKernel* prefix marks them experimental):
- WithKernelTrustedCerts(pem) adds a PEM CA bundle on top of the system roots (for
a re-signing proxy or on-prem CA). Required because the kernel's TLS stack does
not read SSL_CERT_FILE.
- WithKernelSkipHostnameVerify() skips only the hostname check while keeping chain
validation (finer-grained than WithSkipTLSHostVerify).

Setting either without WithUseKernel fails Connect with an error wrapping the
- WithKernelProxy(KernelProxy{URL, Username, Password, BypassHosts}) sets an explicit
HTTP proxy, overriding the HTTP(S)_PROXY / NO_PROXY environment the kernel path
otherwise mirrors. The fields are named (not positional) so the four same-typed
strings can't be transposed at the call site. Use it for the advanced fields the
env-var path can't express: a structured bypass (no-proxy) list, or basic-auth
credentials supplied out of band rather than embedded in the URL. An explicit proxy
takes precedence over the environment; empty credentials / bypass are passed to the
kernel as unset, and a malformed URL is rejected at connect.
- WithKernelRetryOverallTimeout(d) sets the cumulative retry budget across all
attempts — the 4th retry knob, alongside the backoff bounds and max attempts
carried by the backend-neutral WithRetries (also honored on the kernel path). It
is kernel-only because the Thrift WithRetries surface has no overall-budget
equivalent; it mirrors the pyo3/napi retry_overall_timeout knob. Zero keeps the
kernel's default budget (900s).
Comment thread
mani-mathur-arch marked this conversation as resolved.
- WithKernelMaxChunksInMemory(n) bounds how many decompressed CloudFetch chunks the
kernel holds in memory at once — the knob that trades large-result download
throughput for peak RSS. Lower it (e.g. 4) to cap memory on wide, row-heavy result
sets; raise it for more parallelism at higher memory. It is forwarded as the
kernel's client-only cloudfetch_max_chunks_in_memory session conf, stripped before
the SEA wire so it never reaches the server. A value <= 0 keeps the kernel's
default (16).

Setting any of these without WithUseKernel fails Connect with an error wrapping the
sentinel ErrRequiresKernelBackend, detectable with errors.Is.

Features above the backend seam are inherited unchanged: the database/sql connection
pool, per-connection telemetry (CREATE_SESSION / EXECUTE_STATEMENT / DELETE_SESSION),
and the telemetry circuit breaker. Result types render byte-for-byte identical to the
and the telemetry circuit breaker. The kernel path additionally emits a
connection-configuration telemetry event at connect (mode=SEA, auth mechanism/flow,
proxy usage, arrow, query tags, metric-view metadata); this is kernel-only, so the
default (Thrift) path's telemetry is unchanged. Result types render byte-for-byte identical to the
Thrift backend: scalars, DECIMAL (exact string), TIMESTAMP / TIMESTAMP_NTZ (shifted
into the session time zone), INTERVAL, nested Array/Map/Struct and VARIANT (as JSON),
and GEOMETRY (WKT). The server query id is surfaced on the success path, so a
and GEOMETRY / GEOGRAPHY (WKT). The server query id is surfaced on the success path, so a
QueryIdCallback (see below) fires with the real id and EXECUTE_STATEMENT telemetry
carries it.

On the read path, context cancellation is honored at result-batch boundaries, not
mid-fetch: an in-flight CloudFetch batch runs to completion before the cancel takes
effect.

OAuth token caching and HTTP client reuse are inherited from the kernel and need no
driver configuration: the kernel caches U2M tokens on disk
(~/.config/databricks-sql-kernel/oauth/, with refresh-token lifecycle) and M2M
tokens in-memory with background refresh, and reuses a single pooled HTTP client per
session across the control-plane, CloudFetch, and auth-refresh calls. There is no
driver-side cache or pool knob because these live below the C ABI.

# Programmatically Retrieving Connection and Query Id

Use the driverctx package under driverctx/ctx.go to add callbacks to the query context to receive the connection id and query id.
Expand Down
9 changes: 9 additions & 0 deletions errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,15 @@ var ErrKernelNotCompiled error = errors.New("the SEA-via-kernel backend is not c
// the case programmatically instead of matching on message text.
var ErrRequiresKernelBackend error = errors.New("requires the SEA-via-kernel backend")

// value to be used with errors.Is() to determine that a kernel-backend option was
// itself malformed (e.g. a WithKernelProxy URL that does not parse) — as opposed to
// unsupported (ErrNotSupportedByKernel) or a transient connect failure. The kernel
// path validates such options in the Go layer before handing them to the kernel's C
// ABI, where the failure would otherwise surface as an opaque wrapped string a
// caller can't branch on. Lets a caller distinguish "fix your config" from a
// retryable error instead of matching on message text.
var ErrInvalidKernelConfig error = errors.New("invalid kernel backend configuration")

// Base interface for driver errors
type DBError interface {
// Descriptive message describing the error
Expand Down
Loading
Loading