Fix DuckLake ATTACH command format#7
Merged
fuziontech merged 1 commit intomainfrom Dec 4, 2025
Merged
Conversation
- Add AS ducklake alias to ATTACH statement - Remove unused DataPath config option - Update connection string examples to show full format 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Apr 30, 2026
fuziontech
added a commit
that referenced
this pull request
May 1, 2026
…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
added a commit
that referenced
this pull request
May 1, 2026
* refactor(server): extract TLS / ACME helpers into server/tlscert
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>
* refactor(server): extract Arrow Flight client into server/flightclient (#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>
* refactor(server): extract SQL/result interfaces into server/sqlcore so 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>
---------
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
AS ducklakealias to ATTACH statement for proper catalog namingdata_pathconfig optionDetails
The DuckLake ATTACH command was missing the
AS ducklakealias, which is required for the catalog to be properly named. The correct format is:Also removed the
data_pathconfig option which wasn't being used in the connection string format.Test plan
🤖 Generated with Claude Code