From f382ac64a8e694ce4efae1d4debbda239a4c5ad6 Mon Sep 17 00:00:00 2001 From: Joachim Rosskopf Date: Mon, 31 Aug 2026 19:03:12 +0200 Subject: [PATCH] feat(duckvfs): install the extension from the community repository MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The duckvfs lane store could not be deployed. `extension_path` names a LOCALLY BUILT gdrive.duckdb_extension — fine on a developer's machine, impossible in a container — and it is the only way to register write_blob/remove_file/file_size, which live in the extension rather than DuckDB core and are needed for EVERY root, not only `gdrive://`. Leaving it unset does not help, and is the worse failure: `None` SKIPS the load and assumes the functions are present. The store then opens cleanly and the first WRITE fails with "Scalar Function with name write_blob does not exist". A boot that succeeds and a write path that cannot work is exactly the shape that reaches production looking healthy. gdrive now ships in the DuckDB community repository, which duckvfs.rs already anticipated in a comment. `extension_repo` is that: `community`, or a repository URL. Wired through as ESCUREL_STORAGE_DUCKVFS_EXTENSION_REPO. A NEW field rather than repurposing `extension_path: None`. Redefining None would change behaviour for existing `file://` roots and force a network install where none happens today; `extension_path` keeps its exact meaning and still wins, so an operator pointing at a local build gets that build. The test asserts a WRITE round trip, not that the constructor returns Ok: `new` performs no I/O against the root, so a store that never loaded the extension constructs perfectly. It takes no ESCUREL_TEST_GDRIVE_EXTENSION, because needing a prebuilt file is the thing being removed. Two checks, and the second is the one that mattered. Mutating extension_repo to None fails it with the missing-write_blob error, so it is load-bearing. And it first passed in 0.65s — too fast for a download, because gdrive.duckdb_extension was already in ~/.duckdb from local use. Re-run with a cold HOME it downloads, populates the fresh cache and passes, which is the container case; the deployment already sets HOME to a writable dir. Without that check this would have been green here and broken everywhere else. 1377 passed, 0 failed. clippy clean. --- crates/escurel-server/src/config.rs | 13 +++- crates/escurel-storage/src/duckvfs.rs | 39 ++++++++-- .../tests/duckvfs_community_extension.rs | 71 +++++++++++++++++++ .../tests/duckvfs_roundtrip.rs | 1 + 4 files changed, 119 insertions(+), 5 deletions(-) create mode 100644 crates/escurel-storage/tests/duckvfs_community_extension.rs diff --git a/crates/escurel-server/src/config.rs b/crates/escurel-server/src/config.rs index 09b7e7ca..67b085c0 100644 --- a/crates/escurel-server/src/config.rs +++ b/crates/escurel-server/src/config.rs @@ -35,7 +35,8 @@ //! | `ESCUREL_REBUILD_INDEX_ON_BOOT` | `if-missing` | derived-index boot policy: `if-missing` (reuse an existing DuckDB; rebuild only when absent) or `always` (drop + rebuild from the markdown LaneStore each start; the container default — HNSW-persistence-reload workaround) | //! | `ESCUREL_STORAGE_BACKEND` | `fs` | `fs`, `s3`, `gcs` or `duckvfs` | //! | `ESCUREL_STORAGE_DUCKVFS_ROOT` | — | root URL, e.g. `gdrive://escurel/lanes` (backend=duckvfs); its scheme picks the filesystem | -//! | `ESCUREL_STORAGE_DUCKVFS_EXTENSION` | — | path to a built `gdrive.duckdb_extension` (backend=duckvfs); needed for every scheme, not only `gdrive://`, until it ships via the community repo | +//! | `ESCUREL_STORAGE_DUCKVFS_EXTENSION` | — | path to a built `gdrive.duckdb_extension` (backend=duckvfs); needed for every scheme, not only `gdrive://`. Prefer `…_EXTENSION_REPO` — a path is a local build a container does not have | +//! | `ESCUREL_STORAGE_DUCKVFS_EXTENSION_REPO` | — | `community` (the DuckDB community repository) or a repository URL, used when `…_EXTENSION` is unset. Setting NEITHER skips the load rather than failing, so the store opens and the first WRITE fails on a missing `write_blob` | //! | `ESCUREL_STORAGE_DUCKVFS_DRIVE_ID` | — | Shared Drive id `0A…`; REQUIRED for a `gdrive://` root, else the store would silently target the credential's My Drive | //! | `ESCUREL_STORAGE_DUCKVFS_DRIVE_SCOPE` | `…/auth/drive` | OAuth scope; the default is read/write because the extension's own `drive.readonly` default cannot serve a lane store | //! | `ESCUREL_STORAGE_GCS_BUCKET` | — | GCS bucket (backend=gcs) | @@ -554,6 +555,12 @@ pub struct DuckVfsConfig { /// SQL functions the store needs live in it, for every scheme and not /// only `gdrive://`. pub extension_path: Option, + /// Repository to INSTALL the extension from when `extension_path` is + /// unset: `community`, or a repository URL. This is what makes the + /// backend deployable — a path names a local build, which a container + /// does not have, and leaving both unset skips the load rather than + /// failing, so the store opens and the first WRITE fails instead. + pub extension_repo: Option, /// Shared Drive id (`0A…`) for a `gdrive://` root. pub drive_id: Option, /// OAuth scope override; defaults to read/write `drive`. @@ -928,6 +935,9 @@ impl EscurelConfig { extension_path: env .get("ESCUREL_STORAGE_DUCKVFS_EXTENSION") .filter(|v| !v.is_empty()), + extension_repo: env + .get("ESCUREL_STORAGE_DUCKVFS_EXTENSION_REPO") + .filter(|v| !v.is_empty()), drive_id, drive_scope: env .get("ESCUREL_STORAGE_DUCKVFS_DRIVE_SCOPE") @@ -2068,6 +2078,7 @@ impl EscurelConfig { let store = escurel_storage::DuckVfsStore::new(&escurel_storage::DuckVfsStoreConfig { root: cfg.root.clone(), extension_path: cfg.extension_path.clone(), + extension_repo: cfg.extension_repo.clone(), drive_id: cfg.drive_id.clone(), drive_scope: cfg.drive_scope.clone(), }) diff --git a/crates/escurel-storage/src/duckvfs.rs b/crates/escurel-storage/src/duckvfs.rs index bfdc065d..6246a0a2 100644 --- a/crates/escurel-storage/src/duckvfs.rs +++ b/crates/escurel-storage/src/duckvfs.rs @@ -81,9 +81,22 @@ pub struct DuckVfsStoreConfig { /// for a `gdrive://` root and ignored otherwise — but note that the /// `write_blob`/`remove_file`/`move_file`/`file_size` functions live in /// that extension, so **every** root needs it loaded until they ship in - /// DuckDB core. `None` skips the `LOAD` and assumes they are already - /// present. + /// DuckDB core. `None` falls back to [`Self::extension_repo`]; if that + /// is `None` too the `LOAD` is skipped and they are assumed present. pub extension_path: Option, + /// Where to fetch the extension when [`Self::extension_path`] is unset: + /// `"community"` for the DuckDB community repository, or a repository + /// URL such as `http://get.erpl.io`. + /// + /// This is what makes a `duckvfs` store deployable at all. A path names + /// a locally built file, which is fine on a developer's machine and + /// impossible in a container — and falling through to `None` is worse + /// than it looks: it skips the load rather than failing, so the store + /// opens cleanly and the first WRITE fails on a missing function. + /// + /// Ignored when `extension_path` is set, so an operator pointing at a + /// local build always gets that build. + pub extension_repo: Option, /// Shared Drive id (`0A…`) for a `gdrive://` root. Becomes the secret's /// `DRIVE_ID`, which both roots the path and scopes every listing to /// that drive. @@ -100,6 +113,9 @@ pub struct DuckVfsStore { root: String, } +/// The DuckDB community repository, named rather than spelled as a URL. +pub const COMMUNITY_REPO: &str = "community"; + /// Default OAuth scope. `drive.readonly` — the extension's default — is not /// enough for a store that has to write. const DEFAULT_DRIVE_SCOPE: &str = "https://www.googleapis.com/auth/drive"; @@ -129,10 +145,22 @@ impl DuckVfsStore { if let Some(path) = &cfg.extension_path { // A path, not a name: LOAD '' takes the local build - // directly. Once gdrive is in the community repository this - // becomes INSTALL gdrive FROM community; LOAD gdrive; + // directly, and wins over a repository so an operator pointing + // at a build always gets that build. conn.execute_batch(&format!("LOAD '{}';", escape_sql(path))) .map_err(|e| duck_err("load extension", e))?; + } else if let Some(repo) = &cfg.extension_repo { + // `community` is a KEYWORD in DuckDB's grammar and must not be + // quoted; a repository URL must be. Quoting the keyword makes + // DuckDB look for a repository literally named "community" and + // fail with a message that says nothing about quoting. + let install = if repo == COMMUNITY_REPO { + "INSTALL gdrive FROM community;".to_owned() + } else { + format!("INSTALL gdrive FROM '{}';", escape_sql(repo)) + }; + conn.execute_batch(&format!("{install} LOAD gdrive;")) + .map_err(|e| duck_err("install extension", e))?; } if cfg.root.starts_with("gdrive://") { @@ -391,6 +419,7 @@ mod tests { DuckVfsStoreConfig { root: root.to_owned(), extension_path: std::env::var("ESCUREL_TEST_GDRIVE_EXTENSION").ok(), + extension_repo: None, drive_id: None, drive_scope: None, } @@ -430,6 +459,7 @@ mod tests { let cfg = DuckVfsStoreConfig { root: "gdrive://escurel".to_owned(), extension_path: None, + extension_repo: None, drive_id: Some("0AA5vtjzlyjnoUk9PVA".to_owned()), drive_scope: None, }; @@ -450,6 +480,7 @@ mod tests { let cfg = DuckVfsStoreConfig { root: "gdrive://escurel".to_owned(), extension_path: None, + extension_repo: None, drive_id: Some("it's-bad".to_owned()), drive_scope: None, }; diff --git a/crates/escurel-storage/tests/duckvfs_community_extension.rs b/crates/escurel-storage/tests/duckvfs_community_extension.rs new file mode 100644 index 00000000..a4ff0547 --- /dev/null +++ b/crates/escurel-storage/tests/duckvfs_community_extension.rs @@ -0,0 +1,71 @@ +//! The extension can come from the community repository, not only a file. +//! +//! `DuckVfsStoreConfig::extension_path` names a **locally built** +//! `gdrive.duckdb_extension`. That is fine on a developer's machine and +//! impossible in a container: a pod has no such file, and the path is the +//! only way to get `write_blob`/`remove_file`/`file_size` registered — they +//! live in the extension, not in DuckDB core, and every root needs them +//! whatever its scheme. +//! +//! So a `duckvfs` lane store could not be deployed at all. `extension_path: +//! None` does not help: it SKIPS the load and assumes the functions are +//! already present, which in a fresh process they never are. The failure is +//! not at boot either — the store opens happily and the first write fails +//! on a missing function. +//! +//! `gdrive` now ships in the DuckDB community repository, which is what +//! `duckvfs.rs` anticipated in a comment ("Once gdrive is in the community +//! repository this becomes INSTALL gdrive FROM community; LOAD gdrive;"). +//! This is that. +//! +//! Deliberately a `file://` root over a TempDir: the DuckDB VFS dispatches +//! on the scheme, so this exercises the same load path a `gdrive://` root +//! uses while needing no Drive credential. What it proves is that the +//! extension was obtained and its functions registered — nothing about +//! Drive itself, which the live suite covers. +//! +//! Unlike the sibling tests this takes NO `ESCUREL_TEST_GDRIVE_EXTENSION`, +//! because needing a prebuilt file is the very thing being removed. It does +//! need network access to the community repository. + +#![cfg(feature = "duckvfs")] + +use bytes::Bytes; +use escurel_storage::{DuckVfsStore, DuckVfsStoreConfig, Key, LaneStore}; +use tempfile::TempDir; + +fn k(tenant: &str, path: &str) -> Key { + Key::new(tenant.to_owned(), path.to_owned()).expect("key") +} + +/// A store configured with no local extension path still writes and reads. +/// +/// The round trip is the assertion, not the constructor returning `Ok`. +/// `DuckVfsStore::new` performs no I/O against the root, so a store that +/// never loaded the extension constructs perfectly and fails later — which +/// is exactly the shape that would have reached a cluster and presented as +/// a runtime error rather than a boot failure. +#[tokio::test] +async fn the_extension_is_installed_from_the_community_repository() { + let dir = TempDir::new().expect("tempdir"); + let store = DuckVfsStore::new(&DuckVfsStoreConfig { + root: format!("file://{}", dir.path().display()), + extension_path: None, + extension_repo: Some("community".to_owned()), + drive_id: None, + drive_scope: None, + }) + .expect("open a store that sources its extension from the community repo"); + + let key = k("acme", "notes/hello.md"); + let body = Bytes::from_static(b"# hello\n"); + // `write` is the discriminating call: it goes through `write_blob`, + // which lives in the extension rather than DuckDB core. + store + .write(&key, body.clone()) + .await + .expect("write through the community-sourced extension"); + + let got = store.read(&key).await.expect("read back"); + assert_eq!(got, body, "the round trip must return the bytes written"); +} diff --git a/crates/escurel-storage/tests/duckvfs_roundtrip.rs b/crates/escurel-storage/tests/duckvfs_roundtrip.rs index fab44421..c2812d44 100644 --- a/crates/escurel-storage/tests/duckvfs_roundtrip.rs +++ b/crates/escurel-storage/tests/duckvfs_roundtrip.rs @@ -36,6 +36,7 @@ fn store_and_dir() -> Option<(DuckVfsStore, TempDir)> { let store = DuckVfsStore::new(&DuckVfsStoreConfig { root, extension_path: Some(extension_path), + extension_repo: None, drive_id: None, drive_scope: None, })