Skip to content

refactor(server): extract SQL/result interfaces into server/sqlcore so flightclient is duckdb-free#487

Closed
fuziontech wants to merge 1 commit intofeature/binary-split-pr5-flightfrom
feature/binary-split-pr6-sqlcore
Closed

refactor(server): extract SQL/result interfaces into server/sqlcore so flightclient is duckdb-free#487
fuziontech wants to merge 1 commit intofeature/binary-split-pr5-flightfrom
feature/binary-split-pr6-sqlcore

Conversation

@fuziontech
Copy link
Copy Markdown
Member

Summary

  • Step 6 of the binary-split plan, stacked on PR refactor(server): extract Arrow Flight client into server/flightclient #486
  • New server/sqlcore/ subpackage with the SQL/result interfaces (RowSet, ExecResult, RawConn, ColumnTyper, QueryExecutor), IsEmptyQuery + StripLeadingComments helpers, and OTELGRPCClientHandler
  • server/flightclient is now the first consumer to fully drop its server import — making it duckdb-free at the linkage level
  • LocalExecutor / PinnedExecutor / LocalRowSet stay in server/executor.go (standalone-mode-only adapters around *sql.DB, not needed by the CP path)

The win

go list -deps ./server/sqlcore | grep duckdb-go     # empty
go list -deps ./server/flightclient | grep duckdb-go # empty (NEW!)

After this PR, a hypothetical cmd/duckgres-controlplane that imports flightclient plus the other already-extracted leaves (arrowmap, auth, sysinfo, tlscert, ducklake) does not link libduckdb at all. The CP still pulls duckdb-go through its server import for the larger Server / clientConn / Config surface, which PR #7+ tackles.

Backward-compat shims

Type aliases and re-export vars in server/executor.go and a new server/sqlcore_aliases.go keep:

type (
    ColumnTyper   = sqlcore.ColumnTyper
    RowSet        = sqlcore.RowSet
    ExecResult    = sqlcore.ExecResult
    RawConn       = sqlcore.RawConn
    QueryExecutor = sqlcore.QueryExecutor
)

var (
    IsEmptyQuery          = sqlcore.IsEmptyQuery
    OTELGRPCClientHandler = sqlcore.OTELGRPCClientHandler
)

The previously private isEmptyQuery and stripLeadingComments in server/conn.go are now thin one-line wrappers delegating to sqlcore.IsEmptyQuery / sqlcore.StripLeadingComments — so the existing TestIsEmptyQuery / TestStripLeadingComments in conn_test.go pass unchanged.

Test plan

  • go build ./... clean
  • go build -tags kubernetes ./... clean
  • go test -short ./server/sqlcore/... ./server/flightclient/... ./server/... ./controlplane/... — all green
  • go list -deps ./server/sqlcore | grep duckdb-go returns empty
  • go list -deps ./server/flightclient | grep duckdb-go returns empty (the load-bearing check)

Stack

🤖 Generated with Claude Code

@fuziontech fuziontech force-pushed the feature/binary-split-pr5-flight branch from 0451c12 to bc069e4 Compare May 1, 2026 16:04
…o flightclient is duckdb-free

Step 6 of the binary-split plan. Carves the SQL/result interfaces and
two pure helpers out of server/ into a new server/sqlcore subpackage so
the Flight client can talk to those abstractions without importing the
rest of server/ (which still links libduckdb).

Symbols moved:

  - Interfaces: RowSet, ExecResult, RawConn, ColumnTyper, QueryExecutor
    (from server/executor.go)
  - IsEmptyQuery + the previously private isEmptyQuery /
    stripLeadingComments helpers (from server/conn.go and
    server/exports.go) — now sqlcore.IsEmptyQuery and
    sqlcore.StripLeadingComments
  - OTELGRPCClientHandler (from server/otelgrpc_filter.go)

Backward compatibility preserved via type aliases and re-export `var`s
in server/executor.go and a new server/sqlcore_aliases.go. Existing
references to server.RowSet / server.QueryExecutor / server.IsEmptyQuery
/ server.OTELGRPCClientHandler / etc. compile unchanged.

The Flight client (server/flightclient) is the first consumer to drop
its server import entirely. After this PR:

  go list -deps ./server/sqlcore | grep duckdb-go     # empty
  go list -deps ./server/flightclient | grep duckdb-go # empty

This means a hypothetical cmd/duckgres-controlplane that imports
flightclient (and arrowmap, auth, sysinfo, tlscert, ducklake from
earlier PRs) would not link libduckdb at all. The control plane's
remaining duckdb-go linkage comes from its server import for the
larger Server / clientConn / Config surface, which PR #7+ tackles.

LocalExecutor / PinnedExecutor / LocalRowSet stay in server/executor.go
intentionally — they're standalone-mode-only adapters around *sql.DB
and aren't needed by flightclient or the control plane.

Verified:
  - go build ./... clean
  - go build -tags kubernetes ./... clean
  - go test -short ./server/sqlcore/... ./server/flightclient/...
    ./server/... ./controlplane/... — all green (existing
    isEmptyQuery / stripLeadingComments tests in conn_test.go pass via
    the unexported wrappers)
  - go list -deps ./server/sqlcore | grep duckdb-go is empty
  - go list -deps ./server/flightclient | grep duckdb-go is empty

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@fuziontech fuziontech force-pushed the feature/binary-split-pr6-sqlcore branch from 4eff491 to 38c5fb3 Compare May 1, 2026 16:04
@fuziontech fuziontech deleted the branch feature/binary-split-pr5-flight May 1, 2026 16:05
@fuziontech fuziontech closed this May 1, 2026
@fuziontech fuziontech deleted the feature/binary-split-pr6-sqlcore branch May 1, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant