fix: recover persistent catalog WAL after restart - #481
Conversation
Initialize DuckDB with an in-memory default catalog, register MyDuck UDFs, and then attach the persistent catalog so WAL replay can bind generated expressions safely. Keep new sessions on the configured catalog and preserve read-only restart behavior. Cover real process restart, ordinary and generated comment matrices, BLOB vector storage, replication positions, snapshots, concurrency, and provider isolation. Generated through the GitHub platform commit API.
leon-ape
left a comment
There was a problem hiding this comment.
Approved exact signed head a56af4dccce0d40e2b84157a60026098dbfadbc8 only.
Local candidate validation passed: focused catalog restart/WAL replay (in-process and subprocess), provider isolation, and mysql_rand UDF registration stability; race run passed, focused tests passed, and diff-check passed. Scope is limited to catalog/provider.go, catalog/provider_restart_test.go, and catalog/provider_wal_replay_test.go; parent is exact 749b6164b105dafc08d627fa7ab522f4805c654b.
CI attempt 2 packages and Binlog Group 1 (including ServerRestart) passed. Go Query Engine failures match exact-parent run 32628342514/job 97167117220; Binlog Group 2 TestAutoRestartReplica matches exact-parent 32628342547/job 97167117056 (0 -> 1105, unsupported row-event flags 10). Attempt-1 CreateCatalog startup and AutoReconnect timing failures are non-reproducing/exact-base harness behavior per attached waiver. No product blocker found in this PR scope; do not merge or publish from this review.
Root cause
DuckDB 1.5.5 replays
COMMENT ON COLUMNasReplayAlter -> SetColumnComment. When the table contains a generated column, replay rebinds the generated expression before the main database has been installed as DuckDB's default database, aborting startup with:The trigger matrix is specifically generated column + column comment; ordinary tables and generated tables without column comments reopen successfully.
Fix
USEthe configured persistent catalog after the default database exists.Restart(true)with attach-levelREAD_ONLY; in-memory restart retains the prior DSN behavior.This avoids a DuckDB dependency or Doltgres migration and is compatible with generated expressions that reference MyDuck UDFs.
Regression coverage
mysql_randgenerated expression and BLOB-backed VECTOR storage shapeVerification
go test -race -tags duckdb_arrow ./catalog -run '^(TestPersistentCatalogWALReplay|TestPersistentCatalogWALReplayAcrossProcess|TestPersistentCatalogProvidersAreIsolated|TestDatabaseProviderRestartClosesConnectionsBeforeReopen)$' -count=1GOFLAGS=-tags=duckdb_arrow go test ./catalog -count=1GOFLAGS=-tags=duckdb_arrow go test ./... -run '^$' -count=1GOFLAGS=-tags=duckdb_arrow go vet ./catalogCandidate snapshot
749b6164b105dafc08d627fa7ab522f4805c654ba56af4dccce0d40e2b84157a60026098dbfadbc830453d90c23f446354ffa77a84dc9dd8b8e94646catalog/provider.go,catalog/provider_restart_test.go,catalog/provider_wal_replay_test.gogo.mod/go.sum: unchangedDo not merge until independent review completes. The Doltgres v1.2.0 candidate remains separately blocked from merge or release until this fix is reviewed.