Skip to content

Commit 8791b67

Browse files
eddietejedaeddietejeda
committed
refactor(indexes): replace dot/bracket notation with explicit --catalog/--schema/--table/--column flags
Removes the positional `connection.table[col1,col2]` target argument and parse_index_target helper. All index creation now uses named flags, consistent with databases load and search.
1 parent 4feb59c commit 8791b67

2 files changed

Lines changed: 44 additions & 127 deletions

File tree

‎src/command.rs‎

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -329,29 +329,26 @@ pub enum IndexesCommands {
329329
},
330330

331331
/// Create an index on a table or dataset.
332-
///
333-
/// For connection-scoped indexes, pass the table and columns using bracket notation:
334-
/// `connection.table[col1,col2]` or `connection.schema.table[col1,col2]`
335-
/// (schema defaults to `public` when omitted)
336-
///
337-
/// For dataset-scoped indexes, use `--dataset-id` with `--columns`.
338332
Create {
339-
/// Table and columns to index: `connection.table[col1,col2]`
340-
/// or `connection.schema.table[col1,col2]`. Schema defaults to `public`.
341-
///
342-
/// Quote the argument to prevent shell glob expansion:
343-
/// `hotdata indexes create 'airbnb.listings[description]' --type bm25`
344-
#[arg(conflicts_with = "dataset_id")]
345-
target: Option<String>,
333+
/// SQL catalog alias of the target database (e.g. `--catalog airbnb`)
334+
#[arg(long, conflicts_with = "dataset_id")]
335+
catalog: Option<String>,
346336

347-
/// Dataset ID (alternative scope to the positional target)
348-
#[arg(long, conflicts_with = "target")]
349-
dataset_id: Option<String>,
337+
/// Schema name (default: public)
338+
#[arg(long, default_value = "public")]
339+
schema: String,
340+
341+
/// Table name to index
342+
#[arg(long, conflicts_with = "dataset_id")]
343+
table: Option<String>,
350344

351-
/// Columns to index (comma-separated). Required with --dataset-id;
352-
/// for connection scope use bracket notation in the target instead.
345+
/// Column(s) to index (comma-separated)
353346
#[arg(long)]
354-
columns: Option<String>,
347+
column: Option<String>,
348+
349+
/// Dataset ID (alternative scope to --catalog/--table)
350+
#[arg(long, conflicts_with_all = ["catalog", "table"])]
351+
dataset_id: Option<String>,
355352

356353
/// Index name (derived from table, columns, and type if omitted)
357354
#[arg(long)]

‎src/main.rs‎

Lines changed: 28 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -664,9 +664,11 @@ fn main() {
664664
&output,
665665
),
666666
IndexesCommands::Create {
667-
target,
667+
catalog,
668+
schema,
669+
table,
670+
column,
668671
dataset_id,
669-
columns,
670672
name,
671673
r#type,
672674
metric,
@@ -678,46 +680,42 @@ fn main() {
678680
} => {
679681
let api = api::ApiClient::new(Some(&workspace_id));
680682
let (scope, resolved_columns, auto_name) =
681-
match (target.as_deref(), dataset_id.as_deref()) {
682-
(Some(tgt), None) => {
683-
let (conn_name, schema, table, cols) =
684-
parse_index_target(tgt);
685-
let conn_id =
686-
connections::resolve_connection_id(&api, &conn_name);
683+
match (catalog.as_deref().or(table.as_deref()), dataset_id.as_deref()) {
684+
(Some(_), None) => {
685+
let catalog_or_conn = catalog.as_deref().unwrap_or_else(|| {
686+
eprintln!("error: --catalog is required");
687+
std::process::exit(1);
688+
});
689+
let tbl = table.as_deref().unwrap_or_else(|| {
690+
eprintln!("error: --table is required");
691+
std::process::exit(1);
692+
});
693+
let cols = column.as_deref().unwrap_or_else(|| {
694+
eprintln!("error: --column is required");
695+
std::process::exit(1);
696+
});
697+
let conn_id = connections::resolve_connection_id(&api, catalog_or_conn);
687698
let auto = format!(
688-
"{table}_{cols}_{type}",
689-
cols = cols.join("_"),
699+
"{tbl}_{cols}_{type}",
700+
cols = cols.replace(',', "_"),
690701
type = r#type
691702
);
692-
(
693-
(conn_id, schema, table),
694-
cols.join(","),
695-
auto,
696-
)
703+
((conn_id, schema, tbl.to_string()), cols.to_string(), auto)
697704
}
698705
(None, Some(did)) => {
699-
let cols =
700-
columns.as_deref().unwrap_or_else(|| {
701-
eprintln!(
702-
"error: --columns is required with --dataset-id"
703-
);
704-
std::process::exit(1);
705-
});
706+
let cols = column.as_deref().unwrap_or_else(|| {
707+
eprintln!("error: --column is required with --dataset-id");
708+
std::process::exit(1);
709+
});
706710
let auto = format!(
707711
"dataset_{cols}_{type}",
708712
cols = cols.replace(',', "_"),
709713
type = r#type
710714
);
711-
(
712-
(did.to_string(), String::new(), String::new()),
713-
cols.to_string(),
714-
auto,
715-
)
715+
((did.to_string(), String::new(), String::new()), cols.to_string(), auto)
716716
}
717717
_ => {
718-
eprintln!(
719-
"error: provide either <target> (e.g. airbnb.listings[col1,col2]) or --dataset-id with --columns"
720-
);
718+
eprintln!("error: provide --catalog and --table, or --dataset-id with --column");
721719
std::process::exit(1);
722720
}
723721
};
@@ -1065,87 +1063,9 @@ fn main() {
10651063
}
10661064

10671065

1068-
/// Parse an index target like `airbnb.listings[col1,col2]` or
1069-
/// `airbnb.public.listings[col1,col2]` into `(conn_name, schema, table, columns)`.
1070-
/// Schema defaults to `public` when only two dot-parts are given.
1071-
fn parse_index_target(target: &str) -> (String, String, String, Vec<String>) {
1072-
let Some(bracket_pos) = target.find('[') else {
1073-
eprintln!(
1074-
"error: target must include columns in brackets, e.g. airbnb.listings[col1,col2]"
1075-
);
1076-
std::process::exit(1);
1077-
};
1078-
if !target.ends_with(']') {
1079-
eprintln!(
1080-
"error: target bracket is not closed — use e.g. 'airbnb.listings[col1,col2]'"
1081-
);
1082-
std::process::exit(1);
1083-
}
1084-
let table_part = &target[..bracket_pos];
1085-
let cols_raw = &target[bracket_pos + 1..target.len() - 1];
1086-
1087-
let parts: Vec<&str> = table_part.splitn(4, '.').collect();
1088-
let (conn, schema, table) = match parts.as_slice() {
1089-
[c, t] => (c.to_string(), "public".to_string(), t.to_string()),
1090-
[c, s, t] => (c.to_string(), s.to_string(), t.to_string()),
1091-
_ => {
1092-
eprintln!(
1093-
"error: target must be 'connection.table[cols]' or 'connection.schema.table[cols]'"
1094-
);
1095-
std::process::exit(1);
1096-
}
1097-
};
1098-
1099-
let columns: Vec<String> = cols_raw
1100-
.split(',')
1101-
.map(|s| s.trim().to_string())
1102-
.filter(|s| !s.is_empty())
1103-
.collect();
1104-
1105-
if columns.is_empty() {
1106-
eprintln!("error: no columns specified in brackets");
1107-
std::process::exit(1);
1108-
}
1109-
1110-
(conn, schema, table, columns)
1111-
}
1112-
11131066
#[cfg(test)]
11141067
mod tests {
11151068
use super::*;
1116-
1117-
// --- parse_index_target ---
1118-
1119-
#[test]
1120-
fn index_target_two_parts_defaults_schema_to_public() {
1121-
let (conn, schema, table, cols) = parse_index_target("airbnb.listings[description]");
1122-
assert_eq!(conn, "airbnb");
1123-
assert_eq!(schema, "public");
1124-
assert_eq!(table, "listings");
1125-
assert_eq!(cols, vec!["description"]);
1126-
}
1127-
1128-
#[test]
1129-
fn index_target_three_parts_uses_explicit_schema() {
1130-
let (conn, schema, table, cols) =
1131-
parse_index_target("airbnb.public.listings[name,description]");
1132-
assert_eq!(conn, "airbnb");
1133-
assert_eq!(schema, "public");
1134-
assert_eq!(table, "listings");
1135-
assert_eq!(cols, vec!["name", "description"]);
1136-
}
1137-
1138-
#[test]
1139-
fn index_target_multiple_columns() {
1140-
let (_, _, _, cols) = parse_index_target("db.tbl[a,b,c]");
1141-
assert_eq!(cols, vec!["a", "b", "c"]);
1142-
}
1143-
1144-
#[test]
1145-
fn index_target_trims_column_whitespace() {
1146-
let (_, _, _, cols) = parse_index_target("db.tbl[a, b]");
1147-
assert_eq!(cols, vec!["a", "b"]);
1148-
}
11491069
}
11501070

11511071
pub fn get_styles() -> clap::builder::Styles {

0 commit comments

Comments
 (0)