Skip to content

Fix file persistence session initialization - #678

Open
sueun-dev wants to merge 3 commits into
PostHog:mainfrom
sueun-dev:fix-file-persistence-session-init
Open

Fix file persistence session initialization#678
sueun-dev wants to merge 3 commits into
PostHog:mainfrom
sueun-dev:fix-file-persistence-session-init

Conversation

@sueun-dev

Copy link
Copy Markdown

This fixes standalone file-backed connections failing during session metadata initialization when the memory catalog is not attached yet.

What changed:

  • Attach an in-memory memory catalog on the missing-memory-catalog path before installing session metadata views.
  • Restore standalone file-persistence sessions to the persisted physical catalog/search path after metadata setup.
  • Add regression coverage for metadata initialization and a PG-wire reconnect path with file_persistence enabled.

Why:
Issue #561 reports file_persistence: true failing during connection startup with No catalog + schema named "memory" found. The metadata setup needs the compatibility views in memory, but standalone file-backed connections can start without that catalog attached.

Checked:

  • go test -run TestInitSessionDatabaseMetadataWorksOnFileBackedDatabase ./server/sessionmeta
  • go test -run TestFilePersistenceStandaloneConnectionInitializesSession ./server
  • go test ./server/sessionmeta ./server
  • just build
  • just lint
  • just test-unit
  • Not run: Docker-backed integration tests because Docker is not installed in this local environment.

Fixes #561

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8a20ccfbe

ℹ️ 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".

Comment thread server/conn.go Outdated
Comment on lines +1094 to +1095
if c.server.cfg.FilePersistence && !duckLakeAttached && !icebergAttached {
if _, err := c.executor.ExecContext(initCtx, "USE "+sqlcore.QuoteIdentifier(c.username)); err != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Guard file catalog selection on DataDir

When FilePersistence is true but DataDir is empty, DuckDBDSN/openBaseDB intentionally fall back to :memory: (covered by the existing empty-DataDir fallback test), so there is no catalog named after c.username. This new branch still runs USE <username> for that configuration and will turn every standalone connection into a FATAL failed to select file-backed catalog; gate this on an actual file-backed database, e.g. cfg.DataDir != "", so the in-memory fallback continues to work.

Useful? React with 👍 / 👎.

@benben

benben commented Jun 11, 2026

Copy link
Copy Markdown
Member

@sueun-dev thanks for contributing! Can you please take a look at these changes so we get this into a mergeable shape?

  • Rebase: conflicts with main (conn.go split, sessionmeta.go changed)
  • Pass real catalog (c.username), not c.database: views filter on current_database(), so user tables invisible in pg_tables/information_schema.tables, fixes the silently-failing restore-defer too and makes the conn.go hunk
    unnecessary
  • Test: assert created table visible in information_schema.tables
  • Codex comment is valid and should be addressed

@benben benben self-assigned this Jun 11, 2026
… afterwards

InitSessionDatabaseMetadata now gets the file-backed catalog name, so its own
deferred restore lands the session there and the extra USE/search_path block in
conn.go is unnecessary. Gated on DataDir being set, since an empty DataDir makes
DuckDBDSN fall back to :memory: where no catalog is named after the user
(thanks @chatgpt-codex-connector).

The session test now asserts current_database() is the backing catalog and that
a freshly created table shows up in information_schema.tables, which is the
symptom that started this.
@sueun-dev

Copy link
Copy Markdown
Author

Reworked along your notes — thanks for the concrete list.

  • Rebased onto main (merged it in; conn.go and sessionmeta.go both moved).
  • InitSessionDatabaseMetadata now receives the file-backed catalog, so its own deferred restore leaves the session there and the extra USE/search_path block in conn.go is gone. Gated on DataDir != "", since an empty DataDir makes DuckDBDSN fall back to :memory: where there is no catalog named after the user — that was the codex comment, and it was right.
  • The session test asserts a freshly created table is visible in information_schema.tables, and that current_database() is the backing catalog rather than the client's dbname. Reverting the catalog selection makes it fail with current_database() = "test", want "testuser", so it pins the actual symptom.

go test ./server/... is green.

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.

file_persistence: true breaks standalone mode — "No catalog + schema named memory found"

2 participants