refactor: tlscert + flightclient + sqlcore subpackage extractions#488
Merged
fuziontech merged 3 commits intomainfrom May 1, 2026
Merged
refactor: tlscert + flightclient + sqlcore subpackage extractions#488fuziontech merged 3 commits intomainfrom
fuziontech merged 3 commits intomainfrom
Conversation
Step 4 of the binary-split plan. Moves the TLS-cert and ACME (HTTP-01,
DNS-01) helpers out of the server/ package root into a focused
server/tlscert subpackage with no dependency on github.com/duckdb/duckdb-go.
Files moved:
server/certs.go → server/tlscert/certs.go
server/acme.go → server/tlscert/acme.go
server/acme_dns.go → server/tlscert/acme_dns.go
server/acme_test.go → server/tlscert/acme_test.go
server/acme_dns_test.go → server/tlscert/acme_dns_test.go
Backward compatibility preserved via type aliases and re-export `var`s in
server/tlscert_aliases.go:
server.ACMEManager is now an alias for tlscert.ACMEManager
server.ACMEDNSManager is now an alias for tlscert.ACMEDNSManager
server.NewACMEManager / NewACMEDNSManager / EnsureCertificates
are re-export `var`s pointing at tlscert.X
So existing references in main.go, tests/integration/harness.go,
controlplane/control.go, and the Server struct's acmeManager /
acmeDNSManager fields all compile unchanged. New code should import
server/tlscert directly.
The previously-private generateSelfSignedCert is now exported as
tlscert.GenerateSelfSignedCert because server_test.go calls it
directly to generate a cert pair without going through the
EnsureCertificates file-existence check.
PR #4 originally targeted querylog + checkpoint extraction. That hit
a chicken-and-egg with DuckLakeConfig: those constructors take the
server.Config struct, and the duckdb-bound config types haven't moved
to a leaf package yet on this branch (PR #1.5 does that on a parallel
branch). Pivoted to TLS/ACME as a self-contained alternative — same
shape as PR #3, no cycle issues. The querylog / checkpoint move will
land in PR #5 once the config types are sorted.
Verified:
- go build ./... clean
- go build -tags kubernetes ./... clean
- go test -short ./server/tlscert/... ./server/... ./controlplane/... ./
all green
- go list -deps ./server/tlscert | grep duckdb-go is empty
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#486) Step 5 of the binary-split plan. Moves the Arrow Flight SQL client glue (FlightExecutor, FlightRowSet, MaxGRPCMessageSize, the value extraction helpers) out of server/flight_executor.go into a focused server/flightclient subpackage. The package is named flightclient (rather than just flight) to avoid a name collision with arrow-go's github.com/apache/arrow-go/v18/arrow/flight package, which is imported alongside it in flightsqlingress, duckdbservice/service.go, and the controlplane. Files moved: server/flight_executor.go → server/flightclient/flight_executor.go server/flight_executor_test.go → server/flightclient/flight_executor_test.go server/flight_executor_arrow_test.go → server/flightclient/flight_executor_arrow_test.go The previously private intervalValue type moved into arrowmap as IntervalValue (alongside OrderedMapValue) so the result formatters in server/conn.go and server/types.go can switch on it without importing the flightclient subpackage (which would create an import cycle since flightclient imports server for the RowSet/ExecResult/RawConn/ColumnTyper interfaces). External callers updated to import server/flightclient directly: - server/flightsqlingress/ingress.go (and its test) - duckdbservice/service.go - controlplane/{control,session_mgr,flight_ingress,worker_mgr,k8s_pool}.go - controlplane/{control_cancel,session_mgr}_test.go - tests/perf/drivers/flight/driver.go No back-compat aliases in server/ for FlightExecutor / MaxGRPCMessageSize: adding them would create the same import cycle (server -> flightclient -> server). Callers update to flightclient.X directly. There are only a handful, so the migration is mechanical. The formatOrderedMapValue tests moved into a new server/format_ordered_map_test.go because the function under test (server/conn.go) calls into formatValue, which switches on duckdb-go's driver value types — it can't move to a duckdb-free subpackage. The arrowTypeToDuckDB / extractArrowValue tests stayed with their subjects in flightclient. What this PR does NOT achieve: server/flightclient is duckdb-free at the source level, but its transitive import graph still includes duckdb-go because it imports server (for the executor interfaces) and server still links libduckdb. Making server itself duckdb-free is the load-bearing structural work in PR #6+. Verified: - go build ./... clean - go build -tags kubernetes ./... clean - go test -short ./server/flightclient/... ./server/... ./controlplane/... ./duckdbservice/... — all green (pre-existing testcontainer Postgres + integration test failures unrelated) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…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>
This was referenced May 1, 2026
fuziontech
added a commit
that referenced
this pull request
May 1, 2026
…onmeta (#491) Carves the session-local catalog/metadata override helpers out of server/ into a new server/sessionmeta subpackage with no dependency on github.com/duckdb/duckdb-go. The helpers install per-session DuckDB views and macros that make current_database, pg_database, and information_schema_*_compat reflect the client-visible database name on the PG wire — used by both the standalone server's clientConn and the control plane's session bring-up. Symbols moved (from server/session_database_metadata.go to server/sessionmeta/sessionmeta.go): - InitSessionDatabaseMetadata (exported) - HasAttachedCatalog (was private hasAttachedCatalog; now exported) - All the buildSession* / sessionColumn* SQL builder helpers (private) - quoteSQLStringLiteral (private) Type signature change: the QueryExecutor parameter is now sqlcore.QueryExecutor (the interface moved to server/sqlcore in PR #488) instead of server.QueryExecutor. Backward compatibility preserved via re-export `var`s in server/exports.go: var ( HasAttachedCatalog = sessionmeta.HasAttachedCatalog InitSessionDatabaseMetadata = sessionmeta.InitSessionDatabaseMetadata ) Internal callers updated: - server/conn.go now calls sessionmeta.InitSessionDatabaseMetadata and sessionmeta.HasAttachedCatalog directly - controlplane/control.go switches from server.X to sessionmeta.X - server/session_database_metadata{,_remote}_test.go keep their duckdb-go-using mocks but call sessionmeta.X for the moved symbols go list -deps ./server/sessionmeta | grep duckdb-go # empty Verified: - go build ./... clean - go build -tags kubernetes ./... clean - go test -short ./server/... — green except the pre-existing server/tlscert hang on real ACME calls (unrelated) - go list -deps ./server/sessionmeta | grep duckdb-go is empty Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bundle of three previously-stacked PRs (#484 tlscert, #486 flightclient, #487 sqlcore) that got tangled when their stack-children auto-merged into feature-branch bases instead of main, causing GitHub to auto-close the parents. Same exact content, three commits, repackaged as one PR for easier landing.
The three commits in this PR are:
refactor(server): extract TLS / ACME helpers into server/tlscert(was PR refactor(server): extract TLS / ACME helpers into server/tlscert #484)refactor(server): extract Arrow Flight client into server/flightclient(was PR refactor(server): extract Arrow Flight client into server/flightclient #486)refactor(server): extract SQL/result interfaces into server/sqlcore so flightclient is duckdb-free(was PR refactor(server): extract SQL/result interfaces into server/sqlcore so flightclient is duckdb-free #487)Highlights
server.ACMEManager,server.NewACMEManager,server.EnsureCertificates.FlightExecutor,FlightRowSet,MaxGRPCMessageSize, value extractors).intervalValuemoved toarrowmap.IntervalValueso server's result formatters can switch on it without importing flightclient (would create a cycle).RowSet,ExecResult,RawConn,ColumnTyper,QueryExecutor),IsEmptyQuery+StripLeadingCommentshelpers, andOTELGRPCClientHandler.server/flightclientis now the first consumer to fully drop itsserverimport — meaninggo list -deps ./server/flightclient | grep duckdb-goreturns empty.The milestone
A hypothetical CP-only binary that imports flightclient + the existing leaf packages (arrowmap, auth, sysinfo, tlscert, ducklake) does NOT link libduckdb. The CP still pulls duckdb-go through its
serverimport for the larger Server / clientConn / Config surface — the next batch of PRs tackles that.Test plan
go build ./...cleango build -tags kubernetes ./...cleango test -short ./server/... ./controlplane/... ./duckdbservice/...— all greengo list -deps ./server/sqlcore | grep duckdb-goemptygo list -deps ./server/flightclient | grep duckdb-goempty (load-bearing)Stack going forward
🤖 Generated with Claude Code