Skip to content

Commit d75cfc1

Browse files
committed
fix(cli): restore dropped index/table capabilities; ID-based search db refs
Addresses review of #263. Reintroduce capabilities the reshape had accidentally dropped (the shipped skills still documented them): - `search create --type sorted` — sorted indexes (mapped through in create()). - `search create --dimensions/--output-column/--description` — the vector auto-embed options (plumbed into indexes::create). - `databases tables list --table/--limit/--cursor` and the no-active-database workspace-wide listing (restores tables::list); the stale --cursor hint now points at a flag that exists again. Make search's by-name resolution reference the database by id, never a label with fallback: locate_by_name requires a database (explicit --database, or the active one) and carries the db's real default_connection_id (index API) and default_catalog (search SQL). Fixes search remove/show/query failing with a `__db_*` connection label when no active database is set. Adds --database/-d to the search action, `search show`, and `search remove`. Also fix stale --help/hint text: the context no-db hint now says `databases context push`; drop the "also at top-level" parentheticals (only `query` kept a shortcut); `--result-id` help says `hotdata databases results`; README tables-list example.
1 parent 279b5ee commit d75cfc1

8 files changed

Lines changed: 262 additions & 77 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ hotdata databases load --catalog demo --table listings --file ./listings.parquet
176176

177177
```sh
178178
hotdata databases tables list # every queryable table, as connection.schema.table
179-
hotdata databases tables list --connection-id <id> # with column names and types
179+
hotdata databases tables show <table> # column names and types for one table
180180
hotdata query "<sql>" [-o table|json|csv] # PostgreSQL dialect
181181
```
182182

‎src/cli.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,10 @@ pub enum Commands {
106106
#[arg(long, visible_alias = "in")]
107107
index: Option<String>,
108108

109+
/// Database the index lives in (id; defaults to the active database)
110+
#[arg(long, short = 'd')]
111+
database: Option<String>,
112+
109113
/// Columns to display (comma-separated, defaults to all)
110114
#[arg(long)]
111115
select: Option<String>,

‎src/commands/context.rs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::io::Write;
99
use std::path::PathBuf;
1010
use std::sync::LazyLock;
1111

12-
/// Subcommands for `hotdata context`.
12+
/// Subcommands for `hotdata databases context`.
1313
#[derive(clap::Subcommand)]
1414
pub enum ContextCommands {
1515
/// List named contexts in the workspace
@@ -234,8 +234,10 @@ pub fn show(workspace_id: &str, database_id: &str, name: &str) {
234234
);
235235
eprintln!(
236236
"{}",
237-
format!("Create ./{name}.md locally, then run: hotdata context push {name}")
238-
.dark_grey()
237+
format!(
238+
"Create ./{name}.md locally, then run: hotdata databases context push {name}"
239+
)
240+
.dark_grey()
239241
);
240242
std::process::exit(1);
241243
}

‎src/commands/databases.rs‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ pub enum DatabasesCommands {
178178
upload_id: Option<String>,
179179

180180
/// Load a saved query result by id (e.g. `--result-id rslt…`, from
181-
/// `hotdata results` or a query's `[result-id: …]` footer) instead of a
181+
/// `hotdata databases results` or a query's `[result-id: …]` footer) instead of a
182182
/// file. The result must belong to the target database — the one it was
183183
/// queried in.
184184
#[arg(long, conflicts_with_all = ["file", "url", "upload_id"])]
@@ -194,7 +194,7 @@ pub enum DatabasesCommands {
194194
command: Option<DatabaseTablesCommands>,
195195
},
196196

197-
/// Sync database context with local Markdown (also at top-level `context`)
197+
/// Sync database context with local Markdown
198198
Context {
199199
#[command(subcommand)]
200200
command: crate::commands::context::ContextCommands,
@@ -217,7 +217,7 @@ pub enum DatabasesCommands {
217217
command: Option<crate::commands::query::QueryCommands>,
218218
},
219219

220-
/// Inspect query run history (also at top-level `queries`)
220+
/// Inspect query run history
221221
Queries {
222222
/// Query run ID to show details
223223
id: Option<String>,
@@ -234,7 +234,7 @@ pub enum DatabasesCommands {
234234
command: Option<crate::commands::queries::QueriesCommands>,
235235
},
236236

237-
/// Retrieve stored query results (also at top-level `results`)
237+
/// Retrieve stored query results
238238
Results {
239239
/// Result ID (omit to use a subcommand)
240240
result_id: Option<String>,
@@ -261,16 +261,28 @@ pub enum DatabaseTablesCommands {
261261
#[arg(long)]
262262
database: Option<String>,
263263

264-
/// Filter by schema name
264+
/// Filter by schema name (supports % wildcards)
265265
#[arg(long)]
266266
schema: Option<String>,
267267

268+
/// Filter by table name (supports % wildcards)
269+
#[arg(long)]
270+
table: Option<String>,
271+
272+
/// Maximum number of results to return
273+
#[arg(long)]
274+
limit: Option<u32>,
275+
276+
/// Pagination cursor from a previous response
277+
#[arg(long)]
278+
cursor: Option<String>,
279+
268280
/// Output format
269281
#[arg(long = "output", short = 'o', default_value = "table", value_parser = ["table", "json", "yaml"])]
270282
output: String,
271283
},
272284

273-
/// Show column definitions for a table (also at top-level `tables show`)
285+
/// Show column definitions for a table
274286
Show {
275287
/// Table as catalog.schema.table (or schema.table with an active database)
276288
table: String,
@@ -306,7 +318,7 @@ pub enum DatabaseTablesCommands {
306318
upload_id: Option<String>,
307319

308320
/// Load a saved query result by id (e.g. `--result-id rslt…`, from
309-
/// `hotdata results` or a query's `[result-id: …]` footer) instead of a
321+
/// `hotdata databases results` or a query's `[result-id: …]` footer) instead of a
310322
/// file. The result must belong to the target database — the one it was
311323
/// queried in.
312324
#[arg(long, conflicts_with_all = ["file", "url", "upload_id"])]

‎src/commands/indexes.rs‎

Lines changed: 39 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -510,9 +510,12 @@ pub fn create(
510510
}
511511
}
512512

513-
/// A search index located by name, for `search`'s by-name addressing.
513+
/// A search index located by name within a managed database, for `search`'s
514+
/// by-name addressing. Carries the database's real ids (never a `__db_*` label).
514515
pub struct LocatedIndex {
515-
pub connection: String,
516+
pub database_id: String,
517+
pub connection_id: String,
518+
pub catalog: String,
516519
pub schema: String,
517520
pub table: String,
518521
pub index_type: String,
@@ -521,46 +524,55 @@ pub struct LocatedIndex {
521524
pub metric: Option<String>,
522525
}
523526

524-
/// Find a search index by name — scoped to the active managed database when one
525-
/// is set, otherwise across the whole workspace. Errors on no match or on an
526-
/// ambiguous name (present on more than one table).
527-
pub fn locate_by_name(workspace_id: &str, name: &str) -> Result<LocatedIndex, String> {
527+
/// Find a search index by name within a managed database.
528+
///
529+
/// The database is required and addressed by id — an explicit `--database`, or
530+
/// the active one set via `hotdata databases use <id>`. There is no
531+
/// workspace-wide scan and no fallback to the internal `__db_*` connection
532+
/// label: the database's own `default_connection_id` addresses the index API and
533+
/// its `default_catalog` builds search SQL. Errors on no database, no match, or
534+
/// an ambiguous name.
535+
pub fn locate_by_name(
536+
workspace_id: &str,
537+
database: Option<&str>,
538+
name: &str,
539+
) -> Result<LocatedIndex, String> {
528540
let api = Api::new(Some(workspace_id));
529-
// Scope to the active managed database when one is set. Its SQL catalog
530-
// alias is what a query must name — the index listing reports the internal
531-
// `__db_*` connection label, which `bm25_search`/`vector_distance` cannot
532-
// address, so prefer the alias when building the located table below.
533-
let active_db = crate::config::load_current_database("default", workspace_id)
534-
.and_then(|db_id| databases::get_database(&api, &db_id).ok());
535-
let connection_id = active_db
536-
.as_ref()
537-
.map(|db| db.default_connection_id.clone());
538-
let rows = collect_connection_wide(&api, connection_id.as_deref(), None, None)
541+
let db_id = database
542+
.map(str::to_string)
543+
.or_else(|| crate::config::load_current_database("default", workspace_id))
544+
.ok_or_else(|| {
545+
"no database — pass --database <id> or set one with `hotdata databases use <id>`"
546+
.to_string()
547+
})?;
548+
let db = databases::get_database(&api, &db_id).unwrap_or_else(|e| e.exit());
549+
let connection_id = db.default_connection_id;
550+
let catalog = db
551+
.default_catalog
552+
.unwrap_or_else(|| db.name.unwrap_or_else(|| "default".to_string()));
553+
554+
let rows = collect_connection_wide(&api, Some(&connection_id), None, None)
539555
.unwrap_or_else(|e| e.exit());
540556
let matches: Vec<&IndexRow> = rows.iter().filter(|r| r.inner.index_name == name).collect();
541557
match matches.as_slice() {
542558
[] => Err(format!(
543-
"No search index named '{name}' — run 'hotdata search list' to see indexes."
559+
"No search index named '{name}' in this database — run 'hotdata search list' to see indexes."
544560
)),
545561
[one] => {
546562
let loc = one.table.clone().unwrap_or_default();
547563
let parts: Vec<&str> = loc.splitn(3, '.').collect();
548-
let (label_conn, schema, table) = match parts.as_slice() {
549-
[c, s, t] => (c.to_string(), s.to_string(), t.to_string()),
564+
let (schema, table) = match parts.as_slice() {
565+
[_conn, s, t] => (s.to_string(), t.to_string()),
550566
_ => return Err(format!("Could not resolve the table for index '{name}'.")),
551567
};
552-
// Prefer the active db's SQL catalog alias over the internal
553-
// `__db_*` connection label that the index listing reports.
554-
let connection = active_db
555-
.as_ref()
556-
.and_then(|db| db.default_catalog.clone())
557-
.unwrap_or(label_conn);
558568
let search_column = one
559569
.inner
560570
.search_column()
561571
.ok_or_else(|| format!("Index '{name}' has no columns."))?;
562572
Ok(LocatedIndex {
563-
connection,
573+
database_id: db_id,
574+
connection_id,
575+
catalog,
564576
schema,
565577
table,
566578
index_type: one.inner.index_type.clone(),
@@ -570,7 +582,7 @@ pub fn locate_by_name(workspace_id: &str, name: &str) -> Result<LocatedIndex, St
570582
})
571583
}
572584
_ => Err(format!(
573-
"Multiple indexes named '{name}' across tables — this by-name form needs a unique name."
585+
"Multiple indexes named '{name}' on this database — this by-name form needs a unique name."
574586
)),
575587
}
576588
}

0 commit comments

Comments
 (0)