fix(sqlite): preserve SQLite connection URI options (fixes #912)

crates/dataprof-db/src/connectors/sqlite.rs

@@ -69,11 +69,20 @@ impl DatabaseConnector for SqliteConnector {
     async fn connect(&mut self) -> Result<(), DataProfilerError> {
         #[cfg(feature = "sqlite")]
         {
-            let db_path = self.get_db_path()?;
-            let connection_string = if db_path == ":memory:" {
-                "sqlite::memory:".to_string()
+            // sqlx parses SQLite-specific URL options (mode, cache, immutable,
+            // vfs). Passing the original sqlite: URI through unchanged also
+            // preserves its absolute/relative path semantics (#912 / #839).
+            // Previously rebuilding the URL from get_db_path() silently
+            // dropped every query parameter, including mode=ro and mode=rwc.
+            let connection_string = if self.config.connection_string.starts_with("sqlite:") {
+                self.config.connection_string.clone()
             } else {
-                format!("sqlite://{}", db_path)
+                let db_path = self.get_db_path()?;
+                if db_path == ":memory:" {
+                    "sqlite::memory:".to_string()
+                } else {
+                    format!("sqlite://{}", db_path)
+                }
             };
 
             let pool = SqlitePoolOptions::new()

crates/dataprof-db/src/lib.rs

@@ -144,7 +144,10 @@ fn apply_environment_configuration(
                 .apply_to_connection_string(config.connection_string.clone(), database_type);
         }
 
-        if config.load_credentials_from_env {
+        if config.load_credentials_from_env && database_type != "sqlite" {
+            // Explicit SQLite targets are not credentials-bearing URLs. A
+            // generic DATABASE_URL for another engine must not replace the
+            // requested local path or strip its mode=ro/rwc options (#912).
             let credentials = DatabaseCredentials::from_environment(database_type);
             config.connection_string =
                 credentials.apply_to_connection_string(&config.connection_string);

crates/dataprof-db/tests/sqlite_url_options.rs

@@ -0,0 +1,132 @@
+//! #912: SQLite URI query parameters must not be stripped before sqlx sees
+//! them. These are behavioral tests: they exercise actual read-only and
+//! create-if-missing modes, not only string reconstruction.
+#![cfg(feature = "sqlite")]
+
+use dataprof_db::{DatabaseConfig, create_connector};
+use sqlx::sqlite::SqlitePoolOptions;
+
+fn config(url: String) -> DatabaseConfig {
+    DatabaseConfig {
+        connection_string: url,
+        load_credentials_from_env: false,
+        ..Default::default()
+    }
+}
+
+fn sqlite_url(path: &std::path::Path, mode: &str) -> String {
+    // sqlx accepts sqlite:///abs/path on Unix and sqlite:///C:/path on
+    // Windows. A slash is preserved rather than trimming URL path segments.
+    let path = path.to_string_lossy().replace('\\', "/");
+    let prefix = if cfg!(windows) {
+        "sqlite:///"
+    } else {
+        "sqlite://"
+    };
+    format!("{prefix}{path}?mode={mode}")
+}
+
+#[tokio::test]
+async fn mode_rwc_creates_a_missing_database() {
+    let dir = tempfile::tempdir().unwrap();
+    let path = dir.path().join("created-by-mode-rwc.db");
+    assert!(!path.exists());
+
+    let mut connector = create_connector(config(sqlite_url(&path, "rwc"))).unwrap();
+    connector.connect().await.unwrap();
+    assert!(connector.test_connection().await.unwrap());
+    assert!(path.exists(), "SQLite must honor mode=rwc, not drop it");
+    connector.disconnect().await.unwrap();
+}
+
+#[tokio::test]
+async fn mode_rw_does_not_create_a_missing_database() {
+    let dir = tempfile::tempdir().unwrap();
+    let path = dir.path().join("must-not-be-created.db");
+    assert!(!path.exists());
+
+    let mut connector = create_connector(config(sqlite_url(&path, "rw"))).unwrap();
+    assert!(
+        connector.connect().await.is_err(),
+        "mode=rw must reject a missing DB rather than silently create it"
+    );
+    assert!(!path.exists());
+}
+
+#[tokio::test]
+async fn mode_ro_refuses_writes_to_existing_database() {
+    let dir = tempfile::tempdir().unwrap();
+    let path = dir.path().join("read-only.db");
+
+    let pool = SqlitePoolOptions::new()
+        .max_connections(1)
+        .connect(&sqlite_url(&path, "rwc"))
+        .await
+        .unwrap();
+    sqlx::query("CREATE TABLE rows (id INTEGER)")
+        .execute(&pool)
+        .await
+        .unwrap();
+    pool.close().await;
+
+    let mut connector = create_connector(config(sqlite_url(&path, "ro"))).unwrap();
+    connector.connect().await.unwrap();
+    assert!(connector.test_connection().await.unwrap());
+    let error = connector
+        .profile_query("INSERT INTO rows (id) VALUES (1)")
+        .await
+        .expect_err("mode=ro must reject writes to the database");
+    assert!(
+        error.to_string().to_ascii_lowercase().contains("readonly")
+            || error.to_string().to_ascii_lowercase().contains("read-only"),
+        "unexpected SQLite error: {error}"
+    );
+    connector.disconnect().await.unwrap();
+}
+
+/// Execute inside a separate test process: DATABASE_URL is process-global and
+/// must not affect parallel database tests.
+#[tokio::test]
+async fn sqlite_explicit_url_default_env_child() {
+    if std::env::var_os("DATAPROF_SQLITE_ENV_CHILD").is_none() {
+        return;
+    }
+
+    let dir = tempfile::tempdir().unwrap();
+    let path = dir.path().join("explicit-default-rwc.db");
+    let mut connector = create_connector(DatabaseConfig {
+        connection_string: sqlite_url(&path, "rwc"),
+        // Exercise environment loading even if DatabaseConfig defaults change.
+        load_credentials_from_env: true,
+        ..Default::default()
+    })
+    .unwrap();
+    connector.connect().await.unwrap();
+    assert!(path.exists(), "the requested SQLite path must be created");
+    assert!(connector.test_connection().await.unwrap());
+    connector.disconnect().await.unwrap();
+}
+
+#[test]
+fn explicit_sqlite_mode_survives_unrelated_database_url() {
+    let executable = std::env::current_exe().expect("locate Rust test executable");
+    let output = std::process::Command::new(executable)
+        .args([
+            "--exact",
+            "sqlite_explicit_url_default_env_child",
+            "--nocapture",
+        ])
+        .env("DATAPROF_SQLITE_ENV_CHILD", "1")
+        .env(
+            "DATABASE_URL",
+            "postgresql://unrelated:unrelated@127.0.0.1:1/unrelated_database",
+        )
+        .output()
+        .expect("execute isolated default-configuration regression");
+    assert!(
+        output.status.success() && String::from_utf8_lossy(&output.stdout).contains("1 passed"),
+        "an unrelated DATABASE_URL must not hijack an explicit SQLite target:\nstdout:\n{}\nstderr:\n{}",
+        String::from_utf8_lossy(&output.stdout),
+        String::from_utf8_lossy(&output.stderr)
+    );
+}