From 2033c9c35512ef0c7c8ecaa74b309a78b2e9b13a Mon Sep 17 00:00:00 2001 From: Shefeek Jinnah Date: Tue, 30 Jun 2026 21:56:16 +0530 Subject: [PATCH 1/2] fix(indexes): accept --catalog on 'indexes delete' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `indexes create` takes `--catalog ` (resolved to the backing connection, including a managed database's default connection), but `indexes delete` accepted only a raw `--connection-id`. Deleting an index on a managed database therefore required the `default_connection_id` from the original `databases create` response — a value not surfaced by `indexes list` or any `databases` subcommand, so the natural inputs (catalog alias, database id, internal connection name) all failed. Give `delete` the same `--catalog` flag and `resolve_connection_id` resolution that `create` uses; `--connection-id` is retained (mutually exclusive) for the raw-id case. Fixes hotdata-dev/runtimedb#853 --- src/command.rs | 20 +++++++++++++------- src/main.rs | 30 ++++++++++++++++++++---------- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/command.rs b/src/command.rs index 938feb3..57058ee 100644 --- a/src/command.rs +++ b/src/command.rs @@ -356,18 +356,24 @@ pub enum IndexesCommands { /// Delete an index from a table /// - /// Pass connection scope: --connection-id + --schema + --table. + /// Scope the table with `--catalog --schema --table` (the same flag + /// as `indexes create`), or with `--connection-id --schema --table`. Delete { - /// Connection ID (use with --schema and --table) - #[arg(long, short = 'c', requires_all = ["schema", "table"])] + /// SQL catalog alias of the target database (e.g. `--catalog airbnb`) — + /// resolved to the backing connection, the same flag `indexes create` uses + #[arg(long, conflicts_with = "connection_id")] + catalog: Option, + + /// Connection ID (advanced; prefer --catalog) + #[arg(long, short = 'c')] connection_id: Option, - /// Schema name (requires --connection-id) - #[arg(long, requires = "connection_id")] + /// Schema name + #[arg(long)] schema: Option, - /// Table name (requires --connection-id) - #[arg(long, requires = "connection_id")] + /// Table name + #[arg(long)] table: Option, /// Index name diff --git a/src/main.rs b/src/main.rs index 5132446..336d3b9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -657,25 +657,35 @@ fn main() { ) } IndexesCommands::Delete { + catalog, connection_id, schema, table, name, } => { - let scope = match ( - connection_id.as_deref(), - schema.as_deref(), - table.as_deref(), - ) { - (Some(cid), Some(sch), Some(tbl)) => indexes::IndexScope::Connection { - connection_id: cid, + let api = sdk::Api::new(Some(&workspace_id)); + // Resolve the connection the same way `indexes create` + // does: a `--catalog` alias maps to the backing + // connection (incl. a managed database's default + // connection); `--connection-id` is used as-is. + let conn_id = match (catalog.as_deref(), connection_id.as_deref()) { + (Some(cat), _) => connections::resolve_connection_id(&api, cat), + (None, Some(cid)) => cid.to_string(), + (None, None) => { + eprintln!( + "error: provide --catalog (or --connection-id ) with --schema and --table" + ); + std::process::exit(1); + } + }; + let scope = match (schema.as_deref(), table.as_deref()) { + (Some(sch), Some(tbl)) => indexes::IndexScope::Connection { + connection_id: &conn_id, schema: sch, table: tbl, }, _ => { - eprintln!( - "error: provide all three of --connection-id, --schema, --table" - ); + eprintln!("error: --schema and --table are required"); std::process::exit(1); } }; From 07f1d73505edd9673f8635b8ca73b59e1a4b51e8 Mon Sep 17 00:00:00 2001 From: Shefeek Jinnah Date: Tue, 30 Jun 2026 22:24:55 +0530 Subject: [PATCH 2/2] fix(indexes): validate delete scope via clap; default schema to public Review follow-ups for the `indexes delete --catalog` change: - Enforce "exactly one of --catalog / --connection-id" plus a required --table via clap (required_unless_present + conflicts_with), so an incomplete invocation fails locally at the usage exit code (2) BEFORE any auth pre-flight or network resolution, with the requirements shown in --help. The previous runtime check ran after Api::new + connection resolution, so a missing-flag mistake surfaced a misleading auth/connection error and made a needless round-trip. - Default --schema to "public", matching `indexes create` (the doc-comment advertised create-parity but the schema handling diverged). - Build the SDK client only on the --catalog branch; the raw --connection-id path needs no resolution. - Add clap-parse tests covering the delete arg combinations (catalog + default schema, raw connection-id, missing scope, both scopes, missing table). --- src/command.rs | 113 +++++++++++++++++++++++++++++++++++++++++++++---- src/main.rs | 41 +++++++++--------- 2 files changed, 124 insertions(+), 30 deletions(-) diff --git a/src/command.rs b/src/command.rs index 57058ee..dc47fbc 100644 --- a/src/command.rs +++ b/src/command.rs @@ -356,25 +356,29 @@ pub enum IndexesCommands { /// Delete an index from a table /// - /// Scope the table with `--catalog --schema --table` (the same flag - /// as `indexes create`), or with `--connection-id --schema --table`. + /// Scope with `--catalog ` (the same flag as `indexes create`) or + /// `--connection-id `, plus `--table` (and `--schema`, default `public`). Delete { /// SQL catalog alias of the target database (e.g. `--catalog airbnb`) — /// resolved to the backing connection, the same flag `indexes create` uses - #[arg(long, conflicts_with = "connection_id")] + #[arg( + long, + conflicts_with = "connection_id", + required_unless_present = "connection_id" + )] catalog: Option, /// Connection ID (advanced; prefer --catalog) - #[arg(long, short = 'c')] + #[arg(long, short = 'c', required_unless_present = "catalog")] connection_id: Option, - /// Schema name - #[arg(long)] - schema: Option, + /// Schema name (default: public) + #[arg(long, default_value = "public")] + schema: String, /// Table name #[arg(long)] - table: Option, + table: String, /// Index name #[arg(long)] @@ -942,3 +946,96 @@ pub enum EmbeddingProvidersCommands { id: String, }, } + +#[cfg(test)] +mod indexes_delete_arg_tests { + use super::IndexesCommands; + use clap::Parser; + + #[derive(Parser)] + struct Wrapper { + #[command(subcommand)] + cmd: IndexesCommands, + } + + fn parse(args: &[&str]) -> Result { + Wrapper::try_parse_from(std::iter::once("t").chain(args.iter().copied())).map(|w| w.cmd) + } + + #[test] + fn delete_catalog_defaults_schema_to_public() { + let cmd = parse(&[ + "delete", + "--catalog", + "vtest", + "--table", + "hits", + "--name", + "idx", + ]) + .unwrap(); + match cmd { + IndexesCommands::Delete { + catalog, + connection_id, + schema, + table, + name, + } => { + assert_eq!(catalog.as_deref(), Some("vtest")); + assert_eq!(connection_id, None); + assert_eq!(schema, "public"); // defaulted, parity with `create` + assert_eq!(table, "hits"); + assert_eq!(name, "idx"); + } + _ => panic!("expected Delete"), + } + } + + #[test] + fn delete_accepts_raw_connection_id() { + let cmd = parse(&[ + "delete", + "--connection-id", + "conn_x", + "--table", + "hits", + "--name", + "idx", + ]) + .unwrap(); + assert!(matches!( + cmd, + IndexesCommands::Delete { connection_id, catalog: None, .. } if connection_id.as_deref() == Some("conn_x") + )); + } + + #[test] + fn delete_requires_a_scope_flag() { + // neither --catalog nor --connection-id + assert!(parse(&["delete", "--table", "hits", "--name", "idx"]).is_err()); + } + + #[test] + fn delete_rejects_catalog_and_connection_id_together() { + assert!( + parse(&[ + "delete", + "--catalog", + "x", + "--connection-id", + "c", + "--table", + "t", + "--name", + "n" + ]) + .is_err() + ); + } + + #[test] + fn delete_requires_table() { + assert!(parse(&["delete", "--catalog", "x", "--name", "idx"]).is_err()); + } +} diff --git a/src/main.rs b/src/main.rs index 336d3b9..33d2287 100644 --- a/src/main.rs +++ b/src/main.rs @@ -663,33 +663,30 @@ fn main() { table, name, } => { - let api = sdk::Api::new(Some(&workspace_id)); - // Resolve the connection the same way `indexes create` - // does: a `--catalog` alias maps to the backing - // connection (incl. a managed database's default - // connection); `--connection-id` is used as-is. - let conn_id = match (catalog.as_deref(), connection_id.as_deref()) { - (Some(cat), _) => connections::resolve_connection_id(&api, cat), - (None, Some(cid)) => cid.to_string(), + // clap guarantees exactly one of --catalog / --connection-id + // plus --table and --name; --schema defaults to "public". + // A `--catalog` alias resolves to the backing connection + // (incl. a managed database's default connection), the same + // way `indexes create` does; `--connection-id` is used as-is. + let conn_id = match (catalog, connection_id) { + (Some(cat), _) => { + let api = sdk::Api::new(Some(&workspace_id)); + connections::resolve_connection_id(&api, &cat) + } + (None, Some(cid)) => cid, (None, None) => { - eprintln!( - "error: provide --catalog (or --connection-id ) with --schema and --table" - ); - std::process::exit(1); + unreachable!("clap requires --catalog or --connection-id") } }; - let scope = match (schema.as_deref(), table.as_deref()) { - (Some(sch), Some(tbl)) => indexes::IndexScope::Connection { + indexes::delete( + &workspace_id, + indexes::IndexScope::Connection { connection_id: &conn_id, - schema: sch, - table: tbl, + schema: &schema, + table: &table, }, - _ => { - eprintln!("error: --schema and --table are required"); - std::process::exit(1); - } - }; - indexes::delete(&workspace_id, scope, &name); + &name, + ); } } }