Skip to content

Commit 99b1bf9

Browse files
committed
fix(databases): declare --schema beside a dot-qualified --table too
An empty `--table` list was not the only way the chosen schema went undeclared. `--table raw.orders` names its own schema, so with every entry dot-qualified the group list never contained `--schema` either, and `databases create --table raw.orders` left `public` undeclared exactly as a tables-less create used to. A later bare-name declaration into it then fails with "Schema 'public' is not declared" — the same failure, reached a different way, and the doc comment claimed otherwise. Seeding the group list with the chosen schema covers all three cases in one path instead of special-casing the empty one, and a group that collected no tables is emitted without `tables` rather than with an empty array. Found in review of #259.
1 parent 483cc84 commit 99b1bf9

1 file changed

Lines changed: 70 additions & 29 deletions

File tree

‎src/commands/databases.rs‎

Lines changed: 70 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,8 @@ fn schema_name(schema: Option<&str>) -> &str {
561561
/// Build the request body for `POST /v1/databases`.
562562
///
563563
/// The `schema` argument (the `--schema` flag, defaulting to `public`) is always
564-
/// declared, with or without `--table`.
564+
/// declared: with no `--table`, with bare ones, and alongside any schema a
565+
/// dot-qualified `--table` names for itself.
565566
pub fn create_database_request(
566567
name: Option<&str>,
567568
catalog: Option<&str>,
@@ -582,39 +583,47 @@ pub fn create_database_request(
582583
);
583584
}
584585

585-
// `schemas` always goes out, even with no `--table`: the engine only
586-
// auto-declares `main`, so a schemas-less create leaves `--schema` (default
587-
// `public`) undeclared and every later table declaration into it fails with
588-
// "Schema 'public' is not declared". `tables` is optional on a schema
589-
// declaration, so an empty schema is a first-class request.
590-
let schemas_json: Vec<serde_json::Value> = if tables.is_empty() {
591-
vec![serde_json::json!({ "name": schema })]
592-
} else {
593-
// Group tables by schema, preserving insertion order.
594-
// Dot-notation entries (e.g. "raw.raw_orders") use the named schema;
595-
// bare names fall back to the `schema` argument.
596-
let mut schema_tables: Vec<(String, Vec<String>)> = Vec::new();
597-
for t in tables {
598-
let (s, table_name) = match t.split_once('.') {
599-
Some((s, tbl)) => (s.to_string(), tbl.to_string()),
600-
None => (schema.to_string(), t.to_string()),
601-
};
602-
if let Some(entry) = schema_tables.iter_mut().find(|(n, _)| n == &s) {
603-
entry.1.push(table_name);
604-
} else {
605-
schema_tables.push((s, vec![table_name]));
606-
}
586+
// `schemas` always goes out, and always carries `schema`: the engine only
587+
// auto-declares `main`, so leaving `--schema` (default `public`) undeclared
588+
// makes every later table declaration into it fail with "Schema 'public' is
589+
// not declared". `tables` is optional on a schema declaration, so a schema
590+
// with nothing in it yet is a first-class request.
591+
//
592+
// Seeded rather than special-cased on `tables.is_empty()`, because an empty
593+
// `--table` list is not the only way the chosen schema goes undeclared:
594+
// `--table raw.orders` names its own schema, so with every entry
595+
// dot-qualified the group list would never contain `schema` either.
596+
let mut schema_tables: Vec<(String, Vec<String>)> = vec![(schema.to_string(), Vec::new())];
597+
// Dot-notation entries (e.g. "raw.raw_orders") use the named schema; bare
598+
// names fall back to the `schema` argument. Insertion order is preserved,
599+
// and `schema` leads because it is already seeded.
600+
for t in tables {
601+
let (s, table_name) = match t.split_once('.') {
602+
Some((s, tbl)) => (s.to_string(), tbl.to_string()),
603+
None => (schema.to_string(), t.to_string()),
604+
};
605+
if let Some(entry) = schema_tables.iter_mut().find(|(n, _)| n == &s) {
606+
entry.1.push(table_name);
607+
} else {
608+
schema_tables.push((s, vec![table_name]));
607609
}
608-
schema_tables
609-
.into_iter()
610-
.map(|(s, tbls)| {
610+
}
611+
let schemas_json: Vec<serde_json::Value> = schema_tables
612+
.into_iter()
613+
.map(|(s, tbls)| {
614+
// `tables` omitted rather than sent empty: the seeded schema may
615+
// have collected nothing, and the field is optional precisely so a
616+
// schema can be declared on its own.
617+
if tbls.is_empty() {
618+
serde_json::json!({ "name": s })
619+
} else {
611620
serde_json::json!({
612621
"name": s,
613622
"tables": tbls.iter().map(|t| serde_json::json!({ "name": t })).collect::<Vec<_>>()
614623
})
615-
})
616-
.collect()
617-
};
624+
}
625+
})
626+
.collect();
618627
req.insert(
619628
"schemas".to_string(),
620629
serde_json::Value::Array(schemas_json),
@@ -2282,6 +2291,38 @@ mod tests {
22822291
assert_eq!(req["schemas"][0]["name"], "analytics");
22832292
}
22842293

2294+
#[test]
2295+
fn create_database_request_declares_the_chosen_schema_beside_a_dotted_one() {
2296+
// Every --table dot-qualified elsewhere: --schema still has to be
2297+
// declared, or a later bare-name declaration into it fails the same way
2298+
// an empty --table list used to.
2299+
let req = create_database_request(None, None, "public", &["raw.orders".to_string()], None);
2300+
assert_eq!(req["schemas"][0]["name"], "public");
2301+
assert!(
2302+
req["schemas"][0].get("tables").is_none(),
2303+
"the chosen schema collected no tables, so it is declared on its own"
2304+
);
2305+
assert_eq!(req["schemas"][1]["name"], "raw");
2306+
assert_eq!(req["schemas"][1]["tables"][0]["name"], "orders");
2307+
}
2308+
2309+
#[test]
2310+
fn create_database_request_does_not_declare_the_chosen_schema_twice() {
2311+
// A bare name and a dot-qualified one naming the SAME schema are one
2312+
// group, and the seed must not become a second, empty copy of it.
2313+
let req = create_database_request(
2314+
None,
2315+
None,
2316+
"public",
2317+
&["orders".to_string(), "public.customers".to_string()],
2318+
None,
2319+
);
2320+
assert_eq!(req["schemas"].as_array().unwrap().len(), 1);
2321+
assert_eq!(req["schemas"][0]["name"], "public");
2322+
assert_eq!(req["schemas"][0]["tables"][0]["name"], "orders");
2323+
assert_eq!(req["schemas"][0]["tables"][1]["name"], "customers");
2324+
}
2325+
22852326
#[test]
22862327
fn create_database_request_includes_expires_at_when_provided() {
22872328
let req = create_database_request(None, None, "public", &[], Some("24h"));

0 commit comments

Comments
 (0)