Skip to content

Commit ef253bc

Browse files
committed
fix(query): resolve --database name/catalog to id
`hotdata query --database <name>` (or a catalog alias) failed with "Database '<name>' not found" -- only an opaque database id worked -- while `databases tables`, `tables delete --database`, and `databases delete` all accept a name or id. Root cause: query::execute passed the raw --database flag straight to scoped_to_database_opt, which stores it verbatim as the X-Database-Id header value. It never ran the name->id lookup the other subcommands do via resolve_database, so the server rejected any non-id value. Fix: resolve the flag through resolve_database (id-first, then catalog alias, then name) before scoping. Omitting the flag still passes through untouched, preserving the construction-time default database. Added a regression test asserting a --database name resolves to its id and that None passes through without a lookup. Verified end-to-end against the live API: name, catalog, id, and no-flag all work; an unknown value gives the same clean error as the other subcommands.
1 parent 977aec2 commit ef253bc

1 file changed

Lines changed: 52 additions & 1 deletion

File tree

‎src/commands/query.rs‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -439,12 +439,23 @@ fn fail_run(error_msg: &str) -> ! {
439439
std::process::exit(1);
440440
}
441441

442+
/// Resolve an explicit `--database` name/catalog/id to its id for the
443+
/// `X-Database-Id` scope, matching `databases delete`/`tables`. The header
444+
/// requires an id, so a bare name/catalog would otherwise 404 as
445+
/// "Database '<name>' not found". `None` (flag omitted) passes through with no
446+
/// lookup so the construction-time default database is kept.
447+
fn resolve_query_database(api: &Api, database: Option<&str>) -> Option<String> {
448+
database.map(|d| crate::commands::databases::resolve_database(api, d).id)
449+
}
450+
442451
pub fn execute(sql: &str, workspace_id: &str, database: Option<&str>, format: &str) {
443452
// Scope to the explicit --database flag, else the active database resolved
444453
// at construction (HOTDATA_DATABASE / current database). The scoped `Api`
445454
// carries the database into submit_query's `X-Database-Id` header and into
446455
// the database-scoped follow-up fetches (query-run poll, Arrow result).
447-
let api = Api::new(Some(workspace_id)).scoped_to_database_opt(database);
456+
let api = Api::new(Some(workspace_id));
457+
let resolved = resolve_query_database(&api, database);
458+
let api = api.scoped_to_database_opt(resolved.as_deref());
448459
let database = api.database_id();
449460

450461
let mut request = hotdata::models::QueryRequest::new(sql.to_string());
@@ -717,6 +728,46 @@ mod tests {
717728
resp
718729
}
719730

731+
#[test]
732+
fn query_database_flag_resolves_name_to_id() {
733+
// Regression: `hotdata query --database <name>` must resolve a name or
734+
// catalog alias to its id before scoping the X-Database-Id header —
735+
// otherwise the server 404s as "Database '<name>' not found".
736+
let mut server = mockito::Server::new();
737+
// name isn't a valid id → 404, then list matches by name → detail by id.
738+
server
739+
.mock("GET", "/v1/databases/warehouse")
740+
.with_status(404)
741+
.with_body(r#"{"error":"not found"}"#)
742+
.create();
743+
server
744+
.mock("GET", "/v1/databases")
745+
.with_status(200)
746+
.with_header("content-type", "application/json")
747+
.with_body(
748+
r#"{"databases":[{"id":"db_xyz","name":"warehouse","default_catalog":"wh","default_schema":"main"}]}"#,
749+
)
750+
.create();
751+
server
752+
.mock("GET", "/v1/databases/db_xyz")
753+
.with_status(200)
754+
.with_header("content-type", "application/json")
755+
.with_body(
756+
r#"{"id":"db_xyz","name":"warehouse","default_catalog":"wh","default_schema":"main","default_connection_id":"conn_1","attachments":[]}"#,
757+
)
758+
.create();
759+
760+
let api = Api::test_new(&server.url(), "k", Some("ws"));
761+
assert_eq!(
762+
resolve_query_database(&api, Some("warehouse")).as_deref(),
763+
Some("db_xyz"),
764+
"a --database name must resolve to its id"
765+
);
766+
// Flag omitted → no lookup, passes straight through as None so the
767+
// construction-time default database is preserved.
768+
assert_eq!(resolve_query_database(&api, None), None);
769+
}
770+
720771
#[test]
721772
fn hint_for_missing_database_context() {
722773
let tip = cross_source_hint(

0 commit comments

Comments
 (0)