Skip to content

Commit 2db4f7c

Browse files
eddietejedaclaude
andcommitted
fix(search): explicit error on empty index columns; add unit tests for inference logic
- Extract resolve_search_params as a pure function returning Result so the matching logic is testable without API mocking - Replace unwrap_or_default() on columns with ok_or_else() so an index with no columns produces a clear error instead of silent empty string - Add 9 unit tests covering single bm25/vector, hint narrowing, no indexes, multiple indexes, hint type mismatch, and empty columns Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 5135e63 commit 2db4f7c

1 file changed

Lines changed: 151 additions & 47 deletions

File tree

src/indexes.rs

Lines changed: 151 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,61 @@ fn list_one_table_scan(
147147
}
148148
}
149149

150+
/// Pure matching logic for search inference — extracted for testability.
151+
///
152+
/// Filters `indexes` to searchable types (`bm25`, `vector`), narrows by `hint_type` /
153+
/// `hint_column` when provided, and returns `Ok((index_type, column))` on an unambiguous
154+
/// match. Returns `Err(message)` on no match, multiple matches, or an index with no columns.
155+
/// `location` is used only in error messages (e.g. `"mydb.public.listings"`).
156+
fn resolve_search_params(
157+
indexes: &[Index],
158+
hint_type: Option<&str>,
159+
hint_column: Option<&str>,
160+
location: &str,
161+
) -> Result<(String, String), String> {
162+
let matches: Vec<&Index> = indexes
163+
.iter()
164+
.filter(|i| {
165+
let t = i.index_type.as_str();
166+
(t == "bm25" || t == "vector")
167+
&& hint_type.map_or(true, |ht| ht == t)
168+
&& hint_column.map_or(true, |hc| i.columns.iter().any(|c| c == hc))
169+
})
170+
.collect();
171+
172+
match matches.as_slice() {
173+
[] => {
174+
let what = match hint_type {
175+
Some(t) => format!("{} index", t),
176+
None => "BM25 or vector index".to_string(),
177+
};
178+
Err(format!(
179+
"No {} found on {} — run 'hotdata indexes create' first.",
180+
what, location
181+
))
182+
}
183+
[one] => {
184+
let index_type = one.index_type.clone();
185+
let column = one.columns.first().cloned().ok_or_else(|| {
186+
format!("Index '{}' has no columns.", one.index_name)
187+
})?;
188+
Ok((index_type, column))
189+
}
190+
_ => {
191+
let types: Vec<&str> = matches.iter().map(|i| i.index_type.as_str()).collect();
192+
let cols: Vec<String> = matches
193+
.iter()
194+
.flat_map(|i| i.columns.iter().cloned())
195+
.collect();
196+
Err(format!(
197+
"Multiple search indexes found (types: {}, columns: {}) — specify --type and --column.",
198+
types.join(", "),
199+
cols.join(", ")
200+
))
201+
}
202+
}
203+
}
204+
150205
/// Infers `(index_type, column)` for `hotdata search` when `--type` or `--column` are omitted.
151206
///
152207
/// Fetches the indexes on `connection_name.schema.table`, filters to searchable types
@@ -180,53 +235,11 @@ pub fn infer_for_search(
180235
// Fetch indexes for this table
181236
let indexes = list_one_table(&api, &connection_id, schema, table);
182237

183-
// Filter to searchable indexes, honouring any hints
184-
let matches: Vec<&Index> = indexes
185-
.iter()
186-
.filter(|i| {
187-
let t = i.index_type.as_str();
188-
(t == "bm25" || t == "vector")
189-
&& hint_type.map_or(true, |ht| ht == t)
190-
&& hint_column.map_or(true, |hc| i.columns.iter().any(|c| c == hc))
191-
})
192-
.collect();
193-
194-
match matches.as_slice() {
195-
[] => {
196-
let what = match hint_type {
197-
Some(t) => format!("{} index", t),
198-
None => "BM25 or vector index".to_string(),
199-
};
200-
eprintln!(
201-
"{}",
202-
format!(
203-
"No {} found on {}.{}.{} — run 'hotdata indexes create' first.",
204-
what, connection_name, schema, table
205-
)
206-
.red()
207-
);
208-
std::process::exit(1);
209-
}
210-
[one] => {
211-
let index_type = one.index_type.clone();
212-
let column = one.columns.first().cloned().unwrap_or_default();
213-
(index_type, column)
214-
}
215-
_ => {
216-
let types: Vec<&str> = matches.iter().map(|i| i.index_type.as_str()).collect();
217-
let cols: Vec<String> = matches
218-
.iter()
219-
.flat_map(|i| i.columns.iter().cloned())
220-
.collect();
221-
eprintln!(
222-
"{}",
223-
format!(
224-
"Multiple search indexes found (types: {}, columns: {}) — specify --type and --column.",
225-
types.join(", "),
226-
cols.join(", ")
227-
)
228-
.red()
229-
);
238+
let location = format!("{}.{}.{}", connection_name, schema, table);
239+
match resolve_search_params(&indexes, hint_type, hint_column, &location) {
240+
Ok(result) => result,
241+
Err(msg) => {
242+
eprintln!("{}", msg.red());
230243
std::process::exit(1);
231244
}
232245
}
@@ -659,4 +672,95 @@ mod tests {
659672
mock.assert();
660673
assert!(rows.is_empty());
661674
}
675+
676+
fn make_index(name: &str, index_type: &str, columns: &[&str]) -> Index {
677+
Index {
678+
index_name: name.into(),
679+
index_type: index_type.into(),
680+
columns: columns.iter().map(|c| c.to_string()).collect(),
681+
metric: None,
682+
status: "ready".into(),
683+
created_at: "2020-01-01T00:00:00Z".into(),
684+
updated_at: "2020-01-01T00:00:00Z".into(),
685+
}
686+
}
687+
688+
#[test]
689+
fn resolve_search_params_single_bm25_returns_type_and_column() {
690+
let indexes = vec![make_index("fts", "bm25", &["description"])];
691+
let result = resolve_search_params(&indexes, None, None, "db.public.t");
692+
assert_eq!(result, Ok(("bm25".into(), "description".into())));
693+
}
694+
695+
#[test]
696+
fn resolve_search_params_single_vector_returns_type_and_column() {
697+
let indexes = vec![make_index("vec", "vector", &["embedding"])];
698+
let result = resolve_search_params(&indexes, None, None, "db.public.t");
699+
assert_eq!(result, Ok(("vector".into(), "embedding".into())));
700+
}
701+
702+
#[test]
703+
fn resolve_search_params_non_search_indexes_ignored() {
704+
let indexes = vec![
705+
make_index("sorted_idx", "sorted", &["created_at"]),
706+
make_index("fts", "bm25", &["body"]),
707+
];
708+
let result = resolve_search_params(&indexes, None, None, "db.public.t");
709+
assert_eq!(result, Ok(("bm25".into(), "body".into())));
710+
}
711+
712+
#[test]
713+
fn resolve_search_params_hint_type_narrows_to_single() {
714+
let indexes = vec![
715+
make_index("fts", "bm25", &["description"]),
716+
make_index("vec", "vector", &["embedding"]),
717+
];
718+
let result = resolve_search_params(&indexes, Some("bm25"), None, "db.public.t");
719+
assert_eq!(result, Ok(("bm25".into(), "description".into())));
720+
}
721+
722+
#[test]
723+
fn resolve_search_params_hint_column_narrows_to_single() {
724+
let indexes = vec![
725+
make_index("fts_desc", "bm25", &["description"]),
726+
make_index("fts_name", "bm25", &["name"]),
727+
];
728+
let result = resolve_search_params(&indexes, None, Some("name"), "db.public.t");
729+
assert_eq!(result, Ok(("bm25".into(), "name".into())));
730+
}
731+
732+
#[test]
733+
fn resolve_search_params_no_search_indexes_returns_error() {
734+
let indexes = vec![make_index("sorted_idx", "sorted", &["id"])];
735+
let result = resolve_search_params(&indexes, None, None, "db.public.t");
736+
assert!(result.is_err());
737+
assert!(result.unwrap_err().contains("No BM25 or vector index found"));
738+
}
739+
740+
#[test]
741+
fn resolve_search_params_no_index_error_mentions_hint_type() {
742+
let indexes = vec![make_index("fts", "bm25", &["description"])];
743+
let result = resolve_search_params(&indexes, Some("vector"), None, "db.public.t");
744+
assert!(result.is_err());
745+
assert!(result.unwrap_err().contains("vector index"));
746+
}
747+
748+
#[test]
749+
fn resolve_search_params_multiple_matches_returns_error() {
750+
let indexes = vec![
751+
make_index("fts_desc", "bm25", &["description"]),
752+
make_index("fts_name", "bm25", &["name"]),
753+
];
754+
let result = resolve_search_params(&indexes, None, None, "db.public.t");
755+
assert!(result.is_err());
756+
assert!(result.unwrap_err().contains("Multiple search indexes found"));
757+
}
758+
759+
#[test]
760+
fn resolve_search_params_index_with_no_columns_returns_error() {
761+
let indexes = vec![make_index("fts", "bm25", &[])];
762+
let result = resolve_search_params(&indexes, None, None, "db.public.t");
763+
assert!(result.is_err());
764+
assert!(result.unwrap_err().contains("has no columns"));
765+
}
662766
}

0 commit comments

Comments
 (0)