Query DuckLake through direct catalog attachment - #85
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4813d690f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| DuckLakeCatalog: cfg.DuckLakeCatalog, | ||
| DuckLakeCatalogStore: cfg.DuckLakeCatalogStore, | ||
| DuckLakeDataPath: cfg.EffectiveDuckLakeDataPath(), | ||
| DuckLakeDB: duckLakeDB, |
There was a problem hiding this comment.
Do not expose the writer session to arbitrary queries
When DuckLake sync runs with --query-addr, this passes the writer's single DuckDB session to a server whose handleParse executes every client-supplied statement without a read-only restriction. A client can therefore run session-mutating SQL such as DETACH streambed; because the writer uses this exact session, its next fully qualified flush fails with a missing-catalog error and interrupts replication. Keep query clients on an isolated connection or reject statements that can mutate the shared session/catalog.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 606f942. The query server no longer shares the writer DB. It owns a separate DuckDB session, attaches DuckLake with READ_ONLY, and recreates that isolated attachment before every client query so session mutations such as DETACH cannot affect replication and the next query self-heals.
| if querySrv != nil && duckWriter != nil { | ||
| if err := querySrv.SetDuckLakeDB(duckWriter.DB()); err != nil { |
There was a problem hiding this comment.
Swap the query handle before closing the old writer
When a DuckLake pipeline reconnects, the old writer DB is closed at lines 527–529 before this replacement is installed. During that interval, query requests can still acquire duckDBMu.RLock and call QueryContext on the closed handle, producing avoidable client errors on every reconnect; configuring the new session before taking the write lock lengthens the window. Move closing and swapping into one synchronized operation so requests see either the old live handle or the new one.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 606f942 by removing the shared writer/query handle and SetDuckLakeDB path entirely. Pipeline reconnects no longer swap or close a DB used by the query server.
Summary
streambedcatalogstreambed.public.orders,public.orders, andordersnamingiceberg_scan(...)view pathTests
DETACHdoes not affect the writer and the next query self-healsgo test ./...Closes #72