Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
1f29fd7
feat(kernel): add SEA-via-kernel backend (scalar read path)
mani-mathur-arch Jul 9, 2026
8bc7f17
feat(kernel): render nested and complex types
mani-mathur-arch Jul 9, 2026
20950c9
feat(kernel): map TLS, proxy, and session-conf connection options
mani-mathur-arch Jul 9, 2026
0c11201
fix(kernel): honor session timezone; reject unsupported options; fix …
mani-mathur-arch Jul 9, 2026
031c783
fix(kernel): render nested decimals as exact JSON numbers
mani-mathur-arch Jul 9, 2026
8c99345
fix(kernel): sync to merged kernel C ABI (cancel arity, logging, doub…
mani-mathur-arch Jul 10, 2026
d87f614
Merge remote-tracking branch 'origin/main' into mani/sea-kernel-backend
mani-mathur-arch Jul 10, 2026
45aaf84
fix(kernel): map InsecureSkipVerify to both kernel TLS relaxations
mani-mathur-arch Jul 10, 2026
220d0d2
chore(kernel): remove hardcoded local kernel paths from cgo directives
mani-mathur-arch Jul 10, 2026
c7a19f0
fix(kernel): address code review — correctness, doc-contract, parity,…
mani-mathur-arch Jul 10, 2026
5852480
ci(kernel): document deferral of the tagged kernel-path test job (H5)
mani-mathur-arch Jul 10, 2026
a0d1b47
ci(kernel): trim the kernel-path CI deferral comment
mani-mathur-arch Jul 10, 2026
37196b9
docs(kernel): round-2 review — doc/comment/test-helper cleanups
mani-mathur-arch Jul 10, 2026
7d063b2
fix(kernel): round-2 review — float32 parity + close() contract
mani-mathur-arch Jul 10, 2026
0a4ab9b
docs(kernel): round-2 review — correct cancellation + telemetry docs
mani-mathur-arch Jul 10, 2026
490c97d
test(kernel): strengthen proxy test via a resolver seam (N7)
mani-mathur-arch Jul 10, 2026
d7263ea
fix(kernel): remove struct-key prefix cache — unbounded memory leak (N1)
mani-mathur-arch Jul 10, 2026
13a997f
refactor(kernel): extract Arrow cell/JSON rendering to untagged arrow…
mani-mathur-arch Jul 10, 2026
44e180f
fix(arrowscan): suppress G115 on the defensive Uint64 arm
mani-mathur-arch Jul 11, 2026
0955e00
fix(kernel): surface server queryId in kernel error log and message (M5)
mani-mathur-arch Jul 11, 2026
9a5f62d
fix(kernel): close fail-loud contract holes for auth/timeout/retries …
mani-mathur-arch Jul 11, 2026
d272157
refactor(kernel): move proxy resolution to an untagged file so its te…
mani-mathur-arch Jul 11, 2026
bf3b2f1
fix(arrowbased): JSON-escape struct field-name keys + guard cross-bac…
mani-mathur-arch Jul 11, 2026
f90c230
docs(kernel): round-4 review — doc caveats + Thrift grammar back-pointer
mani-mathur-arch Jul 11, 2026
04e12c5
perf(arrowbased): precompute escaped struct keys once, not per row (M1)
mani-mathur-arch Jul 11, 2026
4707b32
fix(arrowscan): render map keys like Thrift (base64 []byte, not fmt %…
mani-mathur-arch Jul 11, 2026
f276304
test(arrowbased): extend cross-backend parity to high-drift nested sh…
mani-mathur-arch Jul 11, 2026
08cbcfe
fix(kernel): never return ErrBadConn on the execute/read path (H1)
mani-mathur-arch Jul 11, 2026
b2f3b57
refactor(kernel): untag config validation + guard against dropped Con…
mani-mathur-arch Jul 11, 2026
e394848
test(kernel): flatten embedded structs in the config drop-guard (M2-r…
mani-mathur-arch Jul 11, 2026
8c730da
fix(kernel): evict a dead session on the statement path; stop retryin…
mani-mathur-arch Jul 11, 2026
5068f5a
perf(arrowscan): memoize struct-key escaping per result set on the ke…
mani-mathur-arch Jul 11, 2026
0eaf600
test(arrowbased): add top-level scalar parity + document the DECIMAL …
mani-mathur-arch Jul 11, 2026
d6a3cf4
cleanup(arrowbased): remove dead fieldNames + strengthen the decimal …
mani-mathur-arch Jul 11, 2026
3a01ead
fix(kernel): round-6 review — int parity, port/protocol reject, untag…
mani-mathur-arch Jul 11, 2026
feb8b2b
docs(kernel): drop internal review-note labels from comments
mani-mathur-arch Jul 13, 2026
cbcd20a
fix(kernel): address review — staging reject, drift guard, typed reje…
mani-mathur-arch Jul 13, 2026
0a14eeb
fix(kernel): address review round 2 — nested-offset correctness + log…
mani-mathur-arch Jul 13, 2026
cd3b1c5
docs(kernel): mark deferred follow-ups inline so reviewers see them
mani-mathur-arch Jul 13, 2026
b895793
fix(kernel): address review round 7 — contract, proxy, cache, message…
mani-mathur-arch Jul 13, 2026
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
6 changes: 6 additions & 0 deletions .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ jobs:
fi
go get -v -t -d ./...

# This matrix builds the default pure-Go driver (CGO_ENABLED=0), which does

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The workflow comment itself states a CGO_ENABLED=1 -tags databricks_kernel job "needs" to be added — confirming no such job exists. All tagged files (backend.go, cgo.go, operation.go, rows.go) compile/run only under the tag, and the only tests touching them are e2e/parity tests that both carry the tag and t.Skip without live creds. So the cancel-watcher goroutine, per-batch release/EOF handling, and close() idempotency never execute in any pipeline. A deadlock/leak/double-free ships green.

  • Suggested fix: add a CGO_ENABLED=1 -tags databricks_kernel job (build-only + a stubbed C ABI for the watcher/close logic) so lifecycle code is exercised without a live warehouse. Track the acknowledged follow-up explicitly.

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

# NOT compile the SEA-via-kernel backend (//go:build cgo && databricks_kernel).
# A dedicated CGO_ENABLED=1 `-tags databricks_kernel` test job needs the
# kernel static library linked in CI, which arrives with the kernel
# distribution work (a pinned-revision source build or a published .a).
# Until then the kernel path's cgo files are not exercised here.
- name: Test
run: make test
env:
Expand Down
46 changes: 42 additions & 4 deletions connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/databricks/databricks-sql-go/auth/pat"
"github.com/databricks/databricks-sql-go/auth/tokenprovider"
"github.com/databricks/databricks-sql-go/driverctx"
"github.com/databricks/databricks-sql-go/internal/backend"
"github.com/databricks/databricks-sql-go/internal/backend/thrift"
"github.com/databricks/databricks-sql-go/internal/client"
"github.com/databricks/databricks-sql-go/internal/config"
Expand All @@ -32,9 +33,18 @@ type connector struct {
func (c *connector) Connect(ctx context.Context) (driver.Conn, error) {
defer debuglog.Track(ctx, "connector.Connect", "host=%s", c.cfg.Host)()

// Build the execution backend. Currently always the Thrift backend; a second
// SEA-via-kernel backend will be selected here by a connect-time flag.
be, err := thrift.New(ctx, c.cfg, c.client)
// Build the execution backend. Thrift is the default; the SEA-via-kernel
// backend is selected when UseKernel is set. newKernelBackend is build-tag
// gated: in the default pure-Go build it returns a clear "not linked in"
// error, so the kernel path compiles and links only under -tags
// databricks_kernel + CGO_ENABLED=1.
var be backend.Backend
var err error
if c.cfg.UseKernel {
be, err = newKernelBackend(ctx, c.cfg)
} else {
be, err = thrift.New(ctx, c.cfg, c.client)
}
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -77,7 +87,14 @@ func (c *connector) Connect(ctx context.Context) (driver.Conn, error) {
conn.telemetry.RecordOperation(ctx, conn.id, "", telemetry.OperationTypeCreateSession, sessionLatencyMs, nil)
}

log.Info().Msgf("connect: host=%s port=%d httpPath=%s serverProtocolVersion=0x%X", c.cfg.Host, c.cfg.Port, c.cfg.HTTPPath, be.ServerProtocolVersion())
// ServerProtocolVersion is Thrift-specific (not on the neutral backend
// interface); the kernel backend has no negotiated Thrift protocol, so log it
// only when present.
if tb, ok := be.(*thrift.Backend); ok {
log.Info().Msgf("connect: host=%s port=%d httpPath=%s serverProtocolVersion=0x%X", c.cfg.Host, c.cfg.Port, c.cfg.HTTPPath, tb.ServerProtocolVersion())
} else {
log.Info().Msgf("connect: host=%s port=%d httpPath=%s backend=kernel", c.cfg.Host, c.cfg.Port, c.cfg.HTTPPath)
}

return conn, nil
}
Expand Down Expand Up @@ -301,6 +318,27 @@ func WithHTTPPath(path string) ConnOption {
}
}

// WithUseKernel selects the SEA-via-kernel backend instead of the default
// Thrift backend. It has effect only in a build compiled with
// `-tags databricks_kernel` and CGO_ENABLED=1; in the default pure-Go build a
// connection made with this option set returns a clear error at connect time
// (the kernel backend is not linked in).
func WithUseKernel(useKernel bool) ConnOption {
return func(c *config.Config) {
c.UseKernel = useKernel
}
}

// WithWarehouseID sets the bare SQL warehouse id. It has no effect unless
// WithUseKernel(true) is also set: the kernel backend addresses a warehouse by id
// (preferred over the http path when set), while the default Thrift backend
// ignores it entirely and continues to route by http path.
func WithWarehouseID(id string) ConnOption {
return func(c *config.Config) {
c.WarehouseID = id
}
}

// WithMaxRows sets up the max rows fetched per request. Default is 10000
func WithMaxRows(n int) ConnOption {
return func(c *config.Config) {
Expand Down
83 changes: 83 additions & 0 deletions doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ Supported optional connection parameters can be specified in param=value and inc
- accessToken: Personal access token. Required if authType set to Pat
- clientID: Specifies the client ID to use with OauthM2M
- clientSecret: Specifies the client secret to use with OauthM2M
- useKernel: Routes execution through the SEA-via-kernel backend instead of Thrift. Requires a build with -tags databricks_kernel (CGO_ENABLED=1); the default pure-Go build returns a clear error. Default is false. See the kernel-backend section below
- warehouseId: The bare SQL warehouse id, used by the kernel backend (which addresses a warehouse by id) in preference to the http path. Ignored by the Thrift backend

Supported optional session parameters can be specified in param=value and include:

Expand Down Expand Up @@ -90,6 +92,8 @@ Supported functional options include:
- WithMaxDownloadThreads (<num_threads> int). Sets up the max number of concurrent workers for cloud fetch. Default is 10. Optional
- WithAuthenticator (<authenticator> auth.Authenticator). Sets up authentication. Required if neither access token or client credentials are provided.
- WithClientCredentials(<clientID> string, <clientSecret> string). Sets up Oauth M2M authentication.
- WithUseKernel(<useKernel> bool). Routes execution through the SEA-via-kernel backend instead of Thrift. Requires a build with -tags databricks_kernel (CGO_ENABLED=1); the default build returns a clear error. Default is false. See the kernel-backend section below. Optional
- WithWarehouseID(<id> string). The bare SQL warehouse id used by the kernel backend in preference to the http path; ignored by the Thrift backend. Optional

# Query cancellation and timeout

Expand Down Expand Up @@ -155,6 +159,74 @@ The result log may look like this:

{"level":"debug","connId":"01ed6545-5669-1ec7-8c7e-6d8a1ea0ab16","corrId":"workflow-example","queryId":"01ed6545-57cc-188a-bfc5-d9c0eaf8e189","time":1668558402,"message":"Run Main elapsed time: 1.298712292s"}

# SEA-via-kernel backend (experimental)

By default the driver uses the Thrift/HiveServer2 backend and is pure Go
(CGO_ENABLED=0, go-gettable, cross-compilable). An experimental second backend
runs statements over the Statement Execution API via the Rust
databricks-sql-kernel, reached through a cgo C ABI. It is opt-in and compiled in
only under a build tag, so the default build is unchanged.

To use it, build with the databricks_kernel tag and CGO enabled, and select it
per connection with WithUseKernel:

CGO_ENABLED=1 go build -tags databricks_kernel ./...

connector, _ := dbsql.NewConnector(
dbsql.WithServerHostname(host),
dbsql.WithHTTPPath(httpPath),
dbsql.WithAccessToken(token),
dbsql.WithUseKernel(true),
)
db := sql.OpenDB(connector)

In a build without the tag, WithUseKernel(true) returns a clear error at connect
time rather than silently using Thrift.

To see the kernel's own logs interleaved with the driver's, set DBSQL_KERNEL_DEBUG
to any non-empty value. That single flag turns on the driver's binding step tracer
AND installs the kernel's Rust log subscriber, so both write to the same stderr in
execution order. It is off by default (and must stay off during benchmarks — debug
logging perturbs latency). The kernel's verbosity is then controlled by RUST_LOG,
which the kernel honors directly; filter on the target databricks::sql::kernel
(note the colons — it is the kernel's explicit log target, NOT the crate module
path databricks_sql_kernel, which would match nothing):

# kernel logs only:
DBSQL_KERNEL_DEBUG=1 RUST_LOG=databricks::sql::kernel=debug ./your_app 2>&1
# kernel logs plus its HTTP stack (hyper/reqwest):
DBSQL_KERNEL_DEBUG=1 RUST_LOG=debug ./your_app 2>&1

The kernel backend currently supports PAT authentication; reading scalar, nested,
and complex-typed results (CloudFetch is handled transparently); context
cancellation during execute (a cancelled ctx fires a real server-side cancel; on
the read path cancellation is honored at result-batch boundaries, not mid-fetch);
and the TLS, proxy, and session-conf (query tags, statement timeout, time zone)
connection options. OAuth (M2M/U2M), initial catalog/schema,
WithEnableMetricViewMetadata, a non-default WithPort, and a non-https protocol
are not yet supported and return a clear error at connect time rather than being
silently ignored (the kernel backend connects over https:443 and has no port or
scheme setter); likewise WithTimeout (a server query timeout the kernel C ABI
can't set) and WithRetries used to disable retries (the kernel retries
internally). Bound query parameters and staging operations
(PUT/GET/REMOVE on a Unity Catalog volume, which need a local file transfer this
backend cannot perform) are likewise not yet supported and return a clear error
at execute time (they are per-statement, not connect-time). None of these is
silently ignored. (Metadata is issued as ordinary SQL —
SHOW/DESCRIBE/information_schema — and runs on this backend like any other
query.)

WithMaxRows and retry tuning (WithRetries with a positive limit) are accepted but
not applied on the kernel path: the kernel manages result fetching and retries
internally, below the C ABI, with no user-facing knob.

Two further kernel-backend caveats. The INTERVAL types (year-month / day-time)
listed in the type table below are not yet handled by the kernel scanner and
return a scan error; use the default (Thrift) backend for interval columns. And
the kernel backend does not yet surface a per-statement server query id, so a
QueryIdCallback (see below) fires with "" and no EXECUTE_STATEMENT telemetry is
emitted for kernel queries.

# 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 Expand Up @@ -226,6 +298,17 @@ Each type has a corresponding sentinel value which can be used with errors.Is()
RequestError
ExecutionError

The kernel backend (WithUseKernel, see above) additionally wraps every rejection
of an option or feature it cannot yet honor with the sentinel ErrNotSupportedByKernel.
A caller can detect this case with errors.Is(err, dbsqlerr.ErrNotSupportedByKernel)
— for example to fall back to the default (Thrift) backend — rather than matching
on the error message text:

if errors.Is(err, dbsqlerr.ErrNotSupportedByKernel) {
// this option/statement isn't supported on the kernel backend yet;
// retry with the default backend (omit WithUseKernel).
}

Example usage:

import (
Expand Down
7 changes: 7 additions & 0 deletions errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ var ExecutionError error = errors.New("Execution Error")
// value to be used with errors.Is() to determine if an error chain contains any databricks error
var DatabricksError error = errors.New("Databricks Error")

// value to be used with errors.Is() to determine if an error is a kernel-backend
// "option/feature not supported" rejection. The kernel backend (WithUseKernel)
// rejects options it cannot yet honor rather than silently ignoring them; wrapping
// them all with this sentinel lets a caller detect the case programmatically (e.g.
// to fall back to the default Thrift backend) instead of matching on message text.
var ErrNotSupportedByKernel error = errors.New("not supported by the kernel backend")

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