Skip to content

Commit 9454c48

Browse files
committed
fix: prefer active database connection when resolving catalog name
1 parent 8791b67 commit 9454c48

4 files changed

Lines changed: 56 additions & 27 deletions

File tree

src/api.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,10 @@ impl ApiClient {
182182
}
183183

184184
/// Test-only client (no config load). Used with a local mock HTTP server.
185+
pub fn workspace_id(&self) -> Option<&str> {
186+
self.workspace_id.as_deref()
187+
}
188+
185189
/// The refresher returns `None`, so 401s are not retried — matching the
186190
/// behavior of tests that don't exercise the refresh path.
187191
#[cfg(test)]

src/connections.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,20 @@ pub fn resolve_connection_id(api: &ApiClient, name_or_id: &str) -> String {
172172
}
173173
}
174174

175+
// Before listing connections, check if the active database's catalog or name
176+
// matches — prefer it over any stale connection entry with the same name.
177+
if let Some(ws) = api.workspace_id() {
178+
if let Some(active_id) = crate::config::load_current_database("default", ws) {
179+
if let Some(active_db) = api.get_none_if_not_found::<crate::databases::Database>(&format!("/databases/{active_id}")) {
180+
if active_db.default_catalog.as_deref() == Some(name_or_id)
181+
|| active_db.name.as_deref() == Some(name_or_id)
182+
{
183+
return active_db.default_connection_id;
184+
}
185+
}
186+
}
187+
}
188+
175189
let body: ListResponse = api.get("/connections");
176190
if let Some(conn) = body
177191
.connections

src/databases.rs

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -111,37 +111,39 @@ pub fn try_resolve_database(api: &ApiClient, id_or_name: &str) -> Result<Databas
111111
return Ok(db);
112112
}
113113

114-
// Fall back to listing and matching by name or catalog alias.
114+
// Fall back to listing — prefer catalog alias match, then name.
115115
let body: ListDatabasesResponse = api.get("/databases");
116-
let name_matches: Vec<&DatabaseSummary> = body
116+
117+
let catalog_matches: Vec<&DatabaseSummary> = body
117118
.databases
118119
.iter()
119-
.filter(|d| d.name.as_deref() == Some(id_or_name))
120+
.filter(|d| d.default_catalog.as_deref() == Some(id_or_name))
120121
.collect();
121122

122-
if !name_matches.is_empty() {
123-
return match name_matches.len() {
124-
1 => Ok(fetch_database(api, &name_matches[0].id)),
123+
if !catalog_matches.is_empty() {
124+
return match catalog_matches.len() {
125+
1 => Ok(fetch_database(api, &catalog_matches[0].id)),
125126
_ => Err(format!(
126-
"multiple databases have name '{}' — use the database id instead",
127+
"multiple databases have catalog '{}' — use the database id instead",
127128
id_or_name
128129
)),
129130
};
130131
}
131132

132-
let catalog_matches: Vec<&DatabaseSummary> = body
133+
134+
let name_matches: Vec<&DatabaseSummary> = body
133135
.databases
134136
.iter()
135-
.filter(|d| d.default_catalog.as_deref() == Some(id_or_name))
137+
.filter(|d| d.name.as_deref() == Some(id_or_name))
136138
.collect();
137139

138-
match catalog_matches.len() {
140+
match name_matches.len() {
139141
0 => Err(format!(
140-
"no database with id, name, or catalog '{id_or_name}'"
142+
"no database with id, catalog, or name '{id_or_name}'"
141143
)),
142-
1 => Ok(fetch_database(api, &catalog_matches[0].id)),
144+
1 => Ok(fetch_database(api, &name_matches[0].id)),
143145
_ => Err(format!(
144-
"multiple databases have catalog '{}' — use the database id instead",
146+
"multiple databases have name '{}' — use the database id instead",
145147
id_or_name
146148
)),
147149
}
@@ -777,7 +779,26 @@ pub fn tables_load(
777779

778780
let database = resolve_current_database(database, workspace_id);
779781
let api = ApiClient::new(Some(workspace_id));
780-
let db = resolve_database(&api, &database);
782+
// Prefer the active database when its catalog or name matches the lookup key,
783+
// avoiding ambiguity when multiple databases share the same catalog name.
784+
let active_id = crate::config::load_current_database("default", workspace_id);
785+
let lookup_key = match active_id.as_deref() {
786+
Some(id) => {
787+
if let Some(active) = api.get_none_if_not_found::<Database>(&format!("/databases/{id}")) {
788+
if active.default_catalog.as_deref() == Some(database.as_str())
789+
|| active.name.as_deref() == Some(database.as_str())
790+
{
791+
id.to_string()
792+
} else {
793+
database.clone()
794+
}
795+
} else {
796+
database.clone()
797+
}
798+
}
799+
None => database.clone(),
800+
};
801+
let db = resolve_database(&api, &lookup_key);
781802
let schema = schema_name(schema);
782803

783804
// clap enforces mutual exclusion; only one of these is ever Some.
@@ -1036,7 +1057,7 @@ mod tests {
10361057

10371058
let api = ApiClient::test_new(&server.url(), "k", None);
10381059
let err = try_resolve_database(&api, "missing").unwrap_err();
1039-
assert!(err.contains("no database with id or name"));
1060+
assert!(err.contains("no database with id"));
10401061
}
10411062

10421063
#[test]

src/indexes.rs

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -219,18 +219,8 @@ pub fn infer_for_search(
219219

220220
let api = ApiClient::new(Some(workspace_id));
221221

222-
// Resolve connection name → ID
223-
let conn_map = connection_lookup(&api);
224-
let connection_id = match conn_map.get(connection_name) {
225-
Some(id) => id.clone(),
226-
None => {
227-
eprintln!(
228-
"{}",
229-
format!("Connection '{}' not found.", connection_name).red()
230-
);
231-
std::process::exit(1);
232-
}
233-
};
222+
// Resolve connection name → ID (falls back to managed database catalog lookup)
223+
let connection_id = crate::connections::resolve_connection_id(&api, connection_name);
234224

235225
// Fetch indexes for this table
236226
let indexes = list_one_table(&api, &connection_id, schema, table);

0 commit comments

Comments
 (0)