From 36ed564dcbbc6ccbfc5b40dee395acb35f0a53a9 Mon Sep 17 00:00:00 2001 From: Wei Cao Date: Mon, 7 Sep 2026 12:26:00 +0800 Subject: [PATCH 1/3] fix: reopen DuckLake catalog after restart and seed data dir ownership Omit CREATE_IF_NOT_EXISTS when the local DuckLake catalog file already exists so same-volume recreate can ATTACH. Create /home/admin/data and /home/admin/log as admin in the image so empty Docker named volumes inherit uid 1000. Document object-table PRIMARY KEY rejection and named-volume ownership. Does not change v0.2.1 / latest. --- catalog/ducklake.go | 16 +++++++++++++--- catalog/provider_ducklake_test.go | 19 +++++++++++++++++++ docker/Dockerfile | 3 +++ docs/object-storage.md | 7 +++++++ 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/catalog/ducklake.go b/catalog/ducklake.go index ea074b4..901dafe 100644 --- a/catalog/ducklake.go +++ b/catalog/ducklake.go @@ -1841,9 +1841,9 @@ func (rt *duckLakeRuntime) attachLocked(ctx context.Context, key any, execer dri } // Both values have already passed configuration validation. SQL-literal // quoting is still required because service paths can contain apostrophes. - attach := "ATTACH IF NOT EXISTS " + duckDBStringLiteral("ducklake:"+metadata) + - " AS " + QuoteIdentifierANSI(DuckLakeCatalogName) + - " (DATA_PATH " + duckDBStringLiteral(dataPath) + ", DATA_INLINING_ROW_LIMIT 0, CREATE_IF_NOT_EXISTS true)" + // CREATE_IF_NOT_EXISTS is only for first attach. Reopening an existing + // local catalog with that option fails DuckDB ATTACH after restart. + attach := duckLakeAttachSQL(metadata, dataPath) if _, err := execer.ExecContext(ctx, attach, nil); err != nil { return newDuckLakeInitError(duckLakeStageAttach, "", err) } @@ -1886,6 +1886,16 @@ func localDuckLakeCatalogPath() (string, error) { return filepath.Join(dir, "ducklake-catalog.duckdb"), nil } +func duckLakeAttachSQL(metadata, dataPath string) string { + opts := "DATA_PATH " + duckDBStringLiteral(dataPath) + ", DATA_INLINING_ROW_LIMIT 0" + if _, err := os.Stat(metadata); err != nil { + opts += ", CREATE_IF_NOT_EXISTS true" + } + return "ATTACH IF NOT EXISTS " + duckDBStringLiteral("ducklake:"+metadata) + + " AS " + QuoteIdentifierANSI(DuckLakeCatalogName) + + " (" + opts + ")" +} + func duckDBStringLiteral(value string) string { // Ordinary DuckDB string literals preserve backslashes; only a single quote // terminates the literal and therefore needs SQL-standard doubling. diff --git a/catalog/provider_ducklake_test.go b/catalog/provider_ducklake_test.go index a8f3aad..0b4b50c 100644 --- a/catalog/provider_ducklake_test.go +++ b/catalog/provider_ducklake_test.go @@ -4,6 +4,8 @@ import ( "context" stdsql "database/sql" "database/sql/driver" + "os" + "path/filepath" "testing" "time" @@ -132,6 +134,23 @@ func TestDuckLakeAttachUsesServicePaths(t *testing.T) { }, execer.queries) } +func TestDuckLakeAttachOmitsCreateIfCatalogExists(t *testing.T) { + dir := t.TempDir() + metadata := filepath.Join(dir, "catalog.ducklake") + require.NoError(t, os.WriteFile(metadata, []byte("existing"), 0o600)) + runtime := &duckLakeRuntime{config: configuration.DuckLakeConfig{ + MetadataPath: metadata, + DataPath: "s3://test-bucket/data", + }} + execer := &recordingDuckLakeExecer{} + + require.NoError(t, runtime.attachLocked(context.Background(), nil, execer)) + require.Equal(t, []string{ + "ATTACH IF NOT EXISTS 'ducklake:" + metadata + "' AS \"__myduck_ducklake\" (DATA_PATH 's3://test-bucket/data', DATA_INLINING_ROW_LIMIT 0)", + }, execer.queries) + require.NotContains(t, execer.queries[0], "CREATE_IF_NOT_EXISTS") +} + func TestDuckLakeAttachRejectsRemoteCatalogURI(t *testing.T) { runtime := &duckLakeRuntime{config: configuration.DuckLakeConfig{ MetadataPath: "s3://test-bucket/catalog.ducklake", diff --git a/docker/Dockerfile b/docker/Dockerfile index ebd8b88..08820be 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -116,6 +116,9 @@ RUN chmod 755 /usr/local/lib/myduckserver /usr/local/lib/myduckserver/duckdb-ext USER admin WORKDIR /home/admin +# Empty Docker named volumes copy this directory's ownership. Without it the +# volume is root:root and admin cannot create myduck.db / ducklake.db. +RUN mkdir -p /home/admin/data /home/admin/log # Copy application files COPY --from=builder /myduckserver /usr/local/bin/myduckserver diff --git a/docs/object-storage.md b/docs/object-storage.md index 0a82493..a1e77dc 100644 --- a/docs/object-storage.md +++ b/docs/object-storage.md @@ -14,6 +14,13 @@ must not include diagnostic inject switches. catalog file (`MYDUCK_DUCKLAKE_METADATA_PATH`). The object bucket stores table data only. Losing the catalog cannot be recovered from the bucket alone. +4. **Object tables do not support PRIMARY KEY.** `CREATE TABLE ... ENGINE=DUCKLAKE` + with a primary key returns 1105. The table is not created and no object + prefix is written. +5. **Data directory ownership.** The image user is `admin` (uid 1000). The + image ships `/home/admin/data` owned by admin so an empty Docker named + volume mounted there inherits that ownership. A bind-mounted host directory + that is `root:root` still needs `chown 1000:1000` before first start. ## Scope From 57058f32c47684a3b5f3c26e6a95806b9bf97483 Mon Sep 17 00:00:00 2001 From: Wei Cao Date: Mon, 7 Sep 2026 12:32:36 +0800 Subject: [PATCH 2/3] fix: do not ATTACH DuckLake catalog as a regular database Restart failed because attachCatalogs attached ducklake.db as a normal DuckDB database, so the later ducklake: ATTACH returned driver_error. Skip the configured metadata file in AttachCatalog. Stat: only ErrNotExist selects CREATE_IF_NOT_EXISTS; permission/IO errors fail closed. CLI ATTACH of the r5 catalog succeeded both with and without CREATE_IF_NOT_EXISTS when the file was not already attached as a regular database. --- catalog/ducklake.go | 21 +++++++++++++++++--- catalog/provider.go | 17 ++++++++++++++++ catalog/provider_ducklake_test.go | 32 +++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 3 deletions(-) diff --git a/catalog/ducklake.go b/catalog/ducklake.go index 901dafe..52d9b98 100644 --- a/catalog/ducklake.go +++ b/catalog/ducklake.go @@ -1843,7 +1843,11 @@ func (rt *duckLakeRuntime) attachLocked(ctx context.Context, key any, execer dri // quoting is still required because service paths can contain apostrophes. // CREATE_IF_NOT_EXISTS is only for first attach. Reopening an existing // local catalog with that option fails DuckDB ATTACH after restart. - attach := duckLakeAttachSQL(metadata, dataPath) + missing, err := duckLakeCatalogMissing(metadata) + if err != nil { + return newDuckLakeInitError(duckLakeStageAttach, "", err) + } + attach := duckLakeAttachSQL(metadata, dataPath, missing) if _, err := execer.ExecContext(ctx, attach, nil); err != nil { return newDuckLakeInitError(duckLakeStageAttach, "", err) } @@ -1886,9 +1890,20 @@ func localDuckLakeCatalogPath() (string, error) { return filepath.Join(dir, "ducklake-catalog.duckdb"), nil } -func duckLakeAttachSQL(metadata, dataPath string) string { +func duckLakeCatalogMissing(path string) (bool, error) { + _, err := os.Stat(path) + if err == nil { + return false, nil + } + if os.IsNotExist(err) { + return true, nil + } + return false, err +} + +func duckLakeAttachSQL(metadata, dataPath string, catalogMissing bool) string { opts := "DATA_PATH " + duckDBStringLiteral(dataPath) + ", DATA_INLINING_ROW_LIMIT 0" - if _, err := os.Stat(metadata); err != nil { + if catalogMissing { opts += ", CREATE_IF_NOT_EXISTS true" } return "ATTACH IF NOT EXISTS " + duckDBStringLiteral("ducklake:"+metadata) + diff --git a/catalog/provider.go b/catalog/provider.go index b494861..f002d84 100644 --- a/catalog/provider.go +++ b/catalog/provider.go @@ -525,6 +525,17 @@ func (prov *DatabaseProvider) HasCatalog(name string) bool { } // attachCatalogs attaches all the databases in the data directory +func (prov *DatabaseProvider) duckLakeMetadataFile(name string) bool { + if prov == nil || prov.duckLake == nil { + return false + } + meta := strings.TrimSpace(prov.duckLake.config.MetadataPath) + if meta == "" { + return false + } + return filepath.Clean(filepath.Join(prov.dataDir, name)) == filepath.Clean(meta) +} + func (prov *DatabaseProvider) attachCatalogs() error { files, err := os.ReadDir(prov.dataDir) if err != nil { @@ -555,6 +566,12 @@ func (prov *DatabaseProvider) AttachCatalog(file interface { } return fmt.Errorf("file %s is not a database file", file.Name()) } + // The DuckLake catalog file lives in dataDir but must not be ATTACHed as a + // regular DuckDB database. Doing so occupies the file and makes the later + // ducklake: ATTACH fail after restart. + if prov.duckLakeMetadataFile(file.Name()) { + return nil + } name := strings.TrimSuffix(file.Name(), ".db") quoted := QuoteIdentifierANSI(name) if _, err := prov.storage.ExecContext(context.Background(), "ATTACH IF NOT EXISTS '"+filepath.Join(prov.dataDir, file.Name())+"' AS "+quoted); err != nil { diff --git a/catalog/provider_ducklake_test.go b/catalog/provider_ducklake_test.go index 0b4b50c..62d4560 100644 --- a/catalog/provider_ducklake_test.go +++ b/catalog/provider_ducklake_test.go @@ -151,6 +151,38 @@ func TestDuckLakeAttachOmitsCreateIfCatalogExists(t *testing.T) { require.NotContains(t, execer.queries[0], "CREATE_IF_NOT_EXISTS") } +func TestAttachCatalogSkipsDuckLakeMetadataFile(t *testing.T) { + dir := t.TempDir() + meta := filepath.Join(dir, "ducklake.db") + require.NoError(t, os.WriteFile(meta, []byte("existing"), 0o600)) + prov := &DatabaseProvider{ + dataDir: dir, + duckLake: &duckLakeRuntime{config: configuration.DuckLakeConfig{ + MetadataPath: meta, + }}, + } + info, err := os.Stat(meta) + require.NoError(t, err) + require.NoError(t, prov.AttachCatalog(info, false)) +} + +func TestDuckLakeAttachStatNonExistErrorDoesNotCreate(t *testing.T) { + dir := t.TempDir() + blocked := filepath.Join(dir, "blocked") + require.NoError(t, os.Mkdir(blocked, 0o000)) + t.Cleanup(func() { _ = os.Chmod(blocked, 0o700) }) + metadata := filepath.Join(blocked, "catalog.ducklake") + runtime := &duckLakeRuntime{config: configuration.DuckLakeConfig{ + MetadataPath: metadata, + DataPath: "s3://test-bucket/data", + }} + execer := &recordingDuckLakeExecer{} + + err := runtime.attachLocked(context.Background(), nil, execer) + require.Error(t, err) + require.Empty(t, execer.queries) +} + func TestDuckLakeAttachRejectsRemoteCatalogURI(t *testing.T) { runtime := &duckLakeRuntime{config: configuration.DuckLakeConfig{ MetadataPath: "s3://test-bucket/catalog.ducklake", From 4be256010814034131cf094b038047434dfc5933 Mon Sep 17 00:00:00 2001 From: Wei Cao Date: Mon, 7 Sep 2026 12:39:44 +0800 Subject: [PATCH 3/3] fix: correct DuckLake restart comments and inject Stat failures Restart failed because attachCatalogs opened the metadata file as a regular DuckDB database. CREATE_IF_NOT_EXISTS is not the recreate cause: CLI ATTACH of the r5 catalog succeeded with and without that option when the file was not already attached. Replace chmod 000 Stat coverage with injected os.ErrPermission so the test is stable as root. --- catalog/ducklake.go | 12 +++++++++--- catalog/provider_ducklake_test.go | 13 +++++++------ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/catalog/ducklake.go b/catalog/ducklake.go index 52d9b98..f06136e 100644 --- a/catalog/ducklake.go +++ b/catalog/ducklake.go @@ -1841,8 +1841,10 @@ func (rt *duckLakeRuntime) attachLocked(ctx context.Context, key any, execer dri } // Both values have already passed configuration validation. SQL-literal // quoting is still required because service paths can contain apostrophes. - // CREATE_IF_NOT_EXISTS is only for first attach. Reopening an existing - // local catalog with that option fails DuckDB ATTACH after restart. + // CREATE_IF_NOT_EXISTS is only for first attach when the catalog file is + // missing. An existing catalog ATTACHes with or without that option. + // Restart failed because attachCatalogs opened the metadata file as a + // regular DuckDB database before the ducklake: ATTACH. missing, err := duckLakeCatalogMissing(metadata) if err != nil { return newDuckLakeInitError(duckLakeStageAttach, "", err) @@ -1890,8 +1892,12 @@ func localDuckLakeCatalogPath() (string, error) { return filepath.Join(dir, "ducklake-catalog.duckdb"), nil } +// duckLakeStat is os.Stat in production. Tests replace it to inject Stat +// failures that chmod 000 cannot produce when the process is root. +var duckLakeStat = os.Stat + func duckLakeCatalogMissing(path string) (bool, error) { - _, err := os.Stat(path) + _, err := duckLakeStat(path) if err == nil { return false, nil } diff --git a/catalog/provider_ducklake_test.go b/catalog/provider_ducklake_test.go index 62d4560..ad118c8 100644 --- a/catalog/provider_ducklake_test.go +++ b/catalog/provider_ducklake_test.go @@ -167,19 +167,20 @@ func TestAttachCatalogSkipsDuckLakeMetadataFile(t *testing.T) { } func TestDuckLakeAttachStatNonExistErrorDoesNotCreate(t *testing.T) { - dir := t.TempDir() - blocked := filepath.Join(dir, "blocked") - require.NoError(t, os.Mkdir(blocked, 0o000)) - t.Cleanup(func() { _ = os.Chmod(blocked, 0o700) }) - metadata := filepath.Join(blocked, "catalog.ducklake") + orig := duckLakeStat + t.Cleanup(func() { duckLakeStat = orig }) + duckLakeStat = func(string) (os.FileInfo, error) { + return nil, os.ErrPermission + } runtime := &duckLakeRuntime{config: configuration.DuckLakeConfig{ - MetadataPath: metadata, + MetadataPath: "/injected/catalog.ducklake", DataPath: "s3://test-bucket/data", }} execer := &recordingDuckLakeExecer{} err := runtime.attachLocked(context.Background(), nil, execer) require.Error(t, err) + require.Contains(t, err.Error(), "reason=permission_denied") require.Empty(t, execer.queries) }