Skip to content

fix: expand SQLite home paths - #579

Open
DhruvShah-Dev wants to merge 1 commit into
TabularisDB:mainfrom
DhruvShah-Dev:fix-sqlite-home-path
Open

fix: expand SQLite home paths#579
DhruvShah-Dev wants to merge 1 commit into
TabularisDB:mainfrom
DhruvShah-Dev:fix-sqlite-home-path

Conversation

@DhruvShah-Dev

Copy link
Copy Markdown
Contributor

Summary

  • Expand leading ~/, ~\, and bare ~ before creating SQLite connection options.
  • Keep relative paths, absolute paths, and ~user paths unchanged.
  • Add focused unit coverage for SQLite filename expansion.

Validation

  • cargo test --lib pool_manager::tests --no-run

Fixes #165

@debba

debba commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

I pulled this down and tested it against real SQLite files in my home directory rather than just reading the diff. Short version: the helper itself is correct and does exactly what it says, but the fix stops one layer short of working, so #165 is still broken after this lands. Details below.

What I ran

Tests, on your branch (that bit matters here, on my own branch they silently don't exist):

cargo test --lib pool_manager::tests   ->  2 passed, 0 failed
cargo test --lib pool_manager          -> 60 passed, 0 failed

Small heads up on the validation line in the description: cargo test --lib pool_manager::tests --no-run only compiles the tests, it never runs them. They do pass, I checked.

Then I staged ~/pr579-db.sqlite (table customers, view customer_names) and ~/pr579dir/data.sqlite (table orders) and drove the real code paths from a throwaway test module:

what I exercised result
pool_manager::get_sqlite_pool("~/pr579-db.sqlite") then SELECT ... FROM sqlite_master works, ["view:customer_names", "table:customers"]
drivers::sqlite::get_tables("~/pr579-db.sqlite"), i.e. what the sidebar calls works, [customers]
drivers::sqlite::get_tables("~/pr579dir/data.sqlite") works, [orders]
baseline: raw ~/... handed straight to sqlx, pre-fix fails, (code: 14) unable to open database file

So the pool manager half genuinely works, and it works everywhere, because every SQLite operation in the driver funnels through get_sqlite_pool. One expansion point covers tables, views, triggers, queries, export. That part I like.

Also params.database.to_string() to params.database.primary() is behaviourally a no-op, since Display for DatabaseSelection is literally write!(f, "{}", self.primary()) (models.rs:103). No hidden change there.

The blocker: the new code never gets reached

test_connection guards file based drivers on the raw path:

// src-tauri/src/commands.rs:2272
if drv.manifest().capabilities.file_based {          // sqlite -> true (drivers/sqlite/mod.rs:990)
    let db_path = std::path::Path::new(resolved_params.database.primary());
    if !db_path.exists() {                           // "~/db.sqlite" -> false, I checked
        return Err(format!("Database file not found: {}", resolved_params.database));

Path::new("~/pr579-db.sqlite").exists() is false, and resolve_connection_params never touches database (for a plain SQLite connection it is a straight params.clone(), commands.rs:385), so the tilde is still there when we hit that check.

And this is not only the Test Connection button, it is the preflight on the normal connect flow:

  • src/contexts/DatabaseProvider.tsx:613-632, opening a saved connection calls test_connection first and on failure it clears the connection state, drops the id from openConnectionIds and rethrows.
  • src/hooks/useOpenConnectionInNewWindow.ts:31, same preflight before spawning a connection window.
  • src/components/modals/NewConnectionModal.tsx:1631, the Test Connection button.

So with this PR merged, a ~/db.sqlite connection still dies with Database file not found: ~/db.sqlite before build_sqlite_connectoptions is ever called. The guard needs the same expansion, which in practice means lifting the helper out of pool_manager (make it pub(crate) somewhere shared) instead of keeping it private.

One more thing worth mentioning in the description: the pre-fix failure is a hard code 14, not the symptom reported in #165 ("successful connection, no tables show up"). So either the reporter's ~/db is a directory, as the first commenter suspected, or the UI is swallowing a connect error and drawing an empty tree, which would be a separate bug. I would go with Refs #165 instead of Fixes #165 until the reporter confirms it is a file.

Smaller things

  1. Tests are in the wrong place, .rules/rust.md (rule 4). src-tauri/src/pool_manager_tests.rs already exists (1023 lines, declared at lib.rs:67, and it already covers SQLite pool creation and pool keys). This PR adds a second test home as an inline #[cfg(test)] mod tests at the bottom of pool_manager.rs. Please move both tests over there and make the helper pub(crate), which you need anyway for the fix above.

  2. normalize_sqlite_path does not expand ~ either. I checked: normalize_sqlite_path("~/pr579-new.db") returns Ok("~/pr579-new.db") (sqlite_database.rs:10), so the "create new SQLite database" flow keeps the literal tilde. That leaves the app with two path handlers that disagree. That one also trim()s its input while expand_sqlite_filename does not, so " ~/db.sqlite" stays literal. A single shared helper fixes both.

  3. drivers/sqlite/mod.rs:1046 still builds sqlite://~/db.sqlite from the raw path. Dead code today, since the only caller is the driver_trait default test_connection and SQLite overrides it at line 1068, but it is a trap for later.

  4. Minor: the bare ~ case resolves to the home directory, which sqlite cannot open anyway, so the test asserts a path that can never work. Harmless, just noise.

  5. directories = "6.0.0" is a direct dependency, all good. The unused variable: options warning at pool_manager.rs:818 is pre-existing from feat(mysql): support AWS RDS IAM authentication #404, not yours.

Where I land

Requesting changes, but only just. The approach is right and I would happily merge it with two things done: expand the path in the file_based existence guard in commands.rs, and move the tests into pool_manager_tests.rs.

For whoever verifies afterwards, here is the setup I used:

sqlite3 ~/pr579-db.sqlite "CREATE TABLE customers(id INTEGER PRIMARY KEY, name TEXT); \
  INSERT INTO customers(name) VALUES('alice'),('bob'); \
  CREATE VIEW customer_names AS SELECT name FROM customers;"
mkdir -p ~/pr579dir && sqlite3 ~/pr579dir/data.sqlite \
  "CREATE TABLE orders(id INTEGER PRIMARY KEY, total REAL); INSERT INTO orders(total) VALUES(9.99);"
database field now after the guard is fixed
~/pr579-db.sqlite Database file not found: ~/pr579-db.sqlite connects, sidebar shows customers and customer_names
~/pr579dir/data.sqlite same error connects, orders with one row
/home/<you>/pr579-db.sqlite works, regression check, must stay working unchanged
~nosuchuser/db.sqlite not found, correct, ~user must not expand unchanged
right click, open in new window, on the ~/ connection fails before the window appears window opens
new connection, create new SQLite database, type ~/pr579-new.db literal tilde path, see point 2 resolves under $HOME

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.

[Bug]: sqlite database at ~/db doesn't load properly

2 participants