Skip to content

Commit 03dc51b

Browse files
authored
feat(databases): load a query result via --result-id (#203)
Add `--result-id` as a fourth source (mutually exclusive with --file / --url / --upload-id) to `databases load` and `databases tables load`: it posts `{ "mode": "replace", "result_id": "<id>" }` to the load endpoint and skips the upload step, publishing a persisted query result into a table. The result must belong to the target database. The server scopes a result-sourced load by the X-Database-Id header, so the standard flow scopes the request to the resolved target database (via scoped_to_database_opt) rather than the ambient active one; uploads keep the ambient scope. The database-API-token flow uses the path-scoped endpoint and needs no header. Skip the delete-and-recreate auto-declare recovery for result loads: it mints a new database id, which would orphan the result (the server scopes it to the original database), and a result load into an undeclared table is auto-declared server-side anyway. Co-authored-by: Eddie A Tejeda <669988+eddietejeda@users.noreply.github.com>
1 parent 37fcca7 commit 03dc51b

2 files changed

Lines changed: 141 additions & 25 deletions

File tree

‎src/commands/databases.rs‎

Lines changed: 137 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ pub enum DatabasesCommands {
107107
name_or_id: String,
108108
},
109109

110-
/// Load a parquet file into a managed database table
110+
/// Load a parquet file or a saved query result into a managed database table
111111
Load {
112112
/// SQL catalog alias of the target database (e.g. `--catalog airbnb`)
113113
#[arg(long)]
@@ -122,16 +122,23 @@ pub enum DatabasesCommands {
122122
table: String,
123123

124124
/// Path to a local parquet file to upload and load
125-
#[arg(long, conflicts_with_all = ["upload_id", "url"])]
125+
#[arg(long, conflicts_with_all = ["upload_id", "url", "result_id"])]
126126
file: Option<String>,
127127

128128
/// URL of a remote parquet file to download and load
129-
#[arg(long, conflicts_with_all = ["file", "upload_id"])]
129+
#[arg(long, conflicts_with_all = ["file", "upload_id", "result_id"])]
130130
url: Option<String>,
131131

132132
/// Use a previously staged upload ID from `POST /v1/uploads` instead of uploading
133-
#[arg(long, conflicts_with_all = ["file", "url"])]
133+
#[arg(long, conflicts_with_all = ["file", "url", "result_id"])]
134134
upload_id: Option<String>,
135+
136+
/// Load a saved query result by id (e.g. `--result-id rslt…`, from
137+
/// `hotdata results` or a query's `[result-id: …]` footer) instead of a
138+
/// file. The result must belong to the target database — the one it was
139+
/// queried in.
140+
#[arg(long, conflicts_with_all = ["file", "url", "upload_id"])]
141+
result_id: Option<String>,
135142
},
136143

137144
/// Manage tables inside a managed database
@@ -190,7 +197,7 @@ pub enum DatabaseTablesCommands {
190197
output: String,
191198
},
192199

193-
/// Load a parquet file into a table (creates or replaces the table)
200+
/// Load a parquet file or a saved query result into a table (creates or replaces the table)
194201
Load {
195202
/// Database id or name (defaults to current database)
196203
#[arg(long)]
@@ -204,16 +211,23 @@ pub enum DatabaseTablesCommands {
204211
schema: String,
205212

206213
/// Path to a local parquet file to upload and load
207-
#[arg(long, conflicts_with_all = ["upload_id", "url"])]
214+
#[arg(long, conflicts_with_all = ["upload_id", "url", "result_id"])]
208215
file: Option<String>,
209216

210217
/// URL of a remote parquet file to download and load
211-
#[arg(long, conflicts_with_all = ["file", "upload_id"])]
218+
#[arg(long, conflicts_with_all = ["file", "upload_id", "result_id"])]
212219
url: Option<String>,
213220

214221
/// Use a previously staged upload ID from `POST /v1/uploads` instead of uploading
215-
#[arg(long, conflicts_with_all = ["file", "url"])]
222+
#[arg(long, conflicts_with_all = ["file", "url", "result_id"])]
216223
upload_id: Option<String>,
224+
225+
/// Load a saved query result by id (e.g. `--result-id rslt…`, from
226+
/// `hotdata results` or a query's `[result-id: …]` footer) instead of a
227+
/// file. The result must belong to the target database — the one it was
228+
/// queried in.
229+
#[arg(long, conflicts_with_all = ["file", "url", "upload_id"])]
230+
result_id: Option<String>,
217231
},
218232

219233
/// Delete a table from a managed database
@@ -561,6 +575,13 @@ pub fn load_table_request(upload_id: &str) -> serde_json::Value {
561575
})
562576
}
563577

578+
pub fn load_table_request_from_result(result_id: &str) -> serde_json::Value {
579+
serde_json::json!({
580+
"mode": "replace",
581+
"result_id": result_id,
582+
})
583+
}
584+
564585
/// Returns true when `path` looks like a parquet file by extension.
565586
pub fn is_parquet_path(path: &str) -> bool {
566587
path.to_ascii_lowercase().ends_with(".parquet")
@@ -1332,6 +1353,7 @@ pub fn tables_list(workspace_id: &str, database: Option<&str>, schema: Option<&s
13321353
}
13331354
}
13341355

1356+
#[allow(clippy::too_many_arguments)]
13351357
pub fn tables_load(
13361358
workspace_id: &str,
13371359
database: Option<&str>,
@@ -1340,6 +1362,7 @@ pub fn tables_load(
13401362
file: Option<&str>,
13411363
url: Option<&str>,
13421364
upload_id: Option<&str>,
1365+
result_id: Option<&str>,
13431366
) {
13441367
use crossterm::style::Stylize;
13451368

@@ -1350,7 +1373,16 @@ pub fn tables_load(
13501373
&& crate::client::credentials::api_key_jwt_source(&profile).as_deref()
13511374
== Some("database_api_token")
13521375
{
1353-
tables_load_database_scoped(workspace_id, database, table, schema, file, url, upload_id);
1376+
tables_load_database_scoped(
1377+
workspace_id,
1378+
database,
1379+
table,
1380+
schema,
1381+
file,
1382+
url,
1383+
upload_id,
1384+
result_id,
1385+
);
13541386
return;
13551387
}
13561388

@@ -1376,22 +1408,31 @@ pub fn tables_load(
13761408
None => database.clone(),
13771409
};
13781410
let db = resolve_database(&api, &lookup_key);
1411+
// A result load hits the connection-scoped endpoint, where the server scopes
1412+
// the result by the X-Database-Id header; that must name the resolved target
1413+
// database, not the ambient active one (which may be unset or different). An
1414+
// upload isn't result-scoped, so it keeps the ambient scope.
1415+
let api = api.scoped_to_database_opt(result_id.map(|_| db.id.as_str()));
13791416
let schema = schema_name(schema);
13801417

1381-
// clap enforces mutual exclusion; only one of these is ever Some.
1382-
let upload_id = match (upload_id, file, url) {
1383-
(Some(id), None, None) => id.to_string(),
1384-
(None, Some(path), None) => upload_parquet_file(&api, path),
1385-
(None, None, Some(u)) => upload_parquet_url(&api, u),
1386-
(None, None, None) => {
1387-
eprintln!("error: --file <path>, --url <url>, or --upload-id <id> is required");
1418+
// clap enforces mutual exclusion; only one source is ever Some. A file/URL is
1419+
// uploaded first and loaded by upload id; a result is loaded by reference with
1420+
// no upload step.
1421+
let body = match (result_id, upload_id, file, url) {
1422+
(Some(rid), None, None, None) => load_table_request_from_result(rid),
1423+
(None, Some(id), None, None) => load_table_request(id),
1424+
(None, None, Some(path), None) => load_table_request(&upload_parquet_file(&api, path)),
1425+
(None, None, None, Some(u)) => load_table_request(&upload_parquet_url(&api, u)),
1426+
(None, None, None, None) => {
1427+
eprintln!(
1428+
"error: one of --file <path>, --url <url>, --upload-id <id>, or --result-id <id> is required"
1429+
);
13881430
std::process::exit(1);
13891431
}
13901432
_ => unreachable!(),
13911433
};
13921434

13931435
let path = managed_table_load_path(&db.default_connection_id, schema, table);
1394-
let body = load_table_request(&upload_id);
13951436

13961437
let spinner = crate::util::spinner("Loading table...");
13971438
let (status, resp_body) = api.post_raw(&path, &body).unwrap_or_else(|e| {
@@ -1401,6 +1442,11 @@ pub fn tables_load(
14011442
spinner.finish_and_clear();
14021443

14031444
let (status, resp_body) = if !status.is_success()
1445+
// Upload-only recovery: the delete + recreate below mints a new database
1446+
// id, which would orphan a result (the server scopes it to the original
1447+
// database). A result load into an undeclared table is auto-declared
1448+
// server-side, so this path isn't needed for it — surface the error.
1449+
&& result_id.is_none()
14041450
&& crate::util::api_error(resp_body.clone()).contains("not declared")
14051451
{
14061452
// The table wasn't declared at create time. Collect existing tables so
@@ -1565,6 +1611,7 @@ pub fn tables_load(
15651611
/// Load path for a database API token: addresses the database by id and uses
15661612
/// the database-scoped endpoints (the connection-scoped managed paths and the
15671613
/// name/catalog resolve used by the standard flow are denied for this token).
1614+
#[allow(clippy::too_many_arguments)]
15681615
fn tables_load_database_scoped(
15691616
workspace_id: &str,
15701617
database: Option<&str>,
@@ -1573,6 +1620,7 @@ fn tables_load_database_scoped(
15731620
file: Option<&str>,
15741621
url: Option<&str>,
15751622
upload_id: Option<&str>,
1623+
result_id: Option<&str>,
15761624
) {
15771625
use crossterm::style::Stylize;
15781626

@@ -1596,20 +1644,25 @@ fn tables_load_database_scoped(
15961644
std::process::exit(1);
15971645
});
15981646

1599-
// clap enforces mutual exclusion; only one of these is ever Some.
1600-
let upload_id = match (upload_id, file, url) {
1601-
(Some(id), None, None) => id.to_string(),
1602-
(None, Some(path), None) => upload_parquet_file(&api, path),
1603-
(None, None, Some(u)) => upload_parquet_url(&api, u),
1604-
(None, None, None) => {
1605-
eprintln!("error: --file <path>, --url <url>, or --upload-id <id> is required");
1647+
// clap enforces mutual exclusion; only one source is ever Some. A file/URL is
1648+
// uploaded first and loaded by upload id; a result is loaded by reference. The
1649+
// database-scoped endpoint scopes the result by the path database id, so no
1650+
// X-Database-Id header is needed here.
1651+
let body = match (result_id, upload_id, file, url) {
1652+
(Some(rid), None, None, None) => load_table_request_from_result(rid),
1653+
(None, Some(id), None, None) => load_table_request(id),
1654+
(None, None, Some(path), None) => load_table_request(&upload_parquet_file(&api, path)),
1655+
(None, None, None, Some(u)) => load_table_request(&upload_parquet_url(&api, u)),
1656+
(None, None, None, None) => {
1657+
eprintln!(
1658+
"error: one of --file <path>, --url <url>, --upload-id <id>, or --result-id <id> is required"
1659+
);
16061660
std::process::exit(1);
16071661
}
16081662
_ => unreachable!(),
16091663
};
16101664

16111665
let load_path = database_table_load_path(&db_id, schema, table);
1612-
let body = load_table_request(&upload_id);
16131666

16141667
let load = || {
16151668
let spinner = crate::util::spinner("Loading table...");
@@ -1960,6 +2013,15 @@ mod tests {
19602013
assert_eq!(body["upload_id"], "upl_abc");
19612014
}
19622015

2016+
#[test]
2017+
fn load_table_request_from_result_is_replace_mode() {
2018+
let body = load_table_request_from_result("rslt_abc");
2019+
assert_eq!(body["mode"], "replace");
2020+
assert_eq!(body["result_id"], "rslt_abc");
2021+
// A result load must not send an upload_id (the server rejects both).
2022+
assert!(body.get("upload_id").is_none());
2023+
}
2024+
19632025
#[test]
19642026
fn is_parquet_path_by_extension() {
19652027
assert!(is_parquet_path("/data/orders.parquet"));
@@ -2097,6 +2159,56 @@ mod tests {
20972159
load.assert();
20982160
}
20992161

2162+
#[test]
2163+
fn tables_load_from_result_posts_result_id_scoped_to_target() {
2164+
let mut server = mockito::Server::new();
2165+
let resolve = server
2166+
.mock("GET", "/v1/databases/db_1")
2167+
.with_status(200)
2168+
.with_header("content-type", "application/json")
2169+
.with_body(full_detail("db_1", "sales", "conn_default"))
2170+
.create();
2171+
// A result load hits the same loads endpoint with a result_id body and no
2172+
// upload step (no POST /v1/uploads mock is registered). The server scopes
2173+
// a result-sourced load by X-Database-Id, so the request must carry the
2174+
// resolved target database id.
2175+
let load = server
2176+
.mock(
2177+
"POST",
2178+
"/v1/connections/conn_default/schemas/public/tables/orders/loads",
2179+
)
2180+
.match_header("X-Database-Id", "db_1")
2181+
.match_body(mockito::Matcher::JsonString(
2182+
serde_json::to_string(&load_table_request_from_result("rslt_123")).unwrap(),
2183+
))
2184+
.with_status(200)
2185+
.with_body(
2186+
r#"{
2187+
"connection_id":"conn_default",
2188+
"schema_name":"public",
2189+
"table_name":"orders",
2190+
"row_count":42,
2191+
"arrow_schema_json":"{}"
2192+
}"#,
2193+
)
2194+
.create();
2195+
2196+
let api = Api::test_new(&server.url(), "k", Some("ws1"));
2197+
let db = resolve_database(&api, "db_1");
2198+
// Mirror tables_load: scope to the resolved target database so the load
2199+
// carries X-Database-Id.
2200+
let api = api.scoped_to_database_opt(Some(db.id.as_str()));
2201+
let path = managed_table_load_path(&db.default_connection_id, "public", "orders");
2202+
let body = load_table_request_from_result("rslt_123");
2203+
let (status, resp_body) = api.post_raw(&path, &body).unwrap();
2204+
assert!(status.is_success());
2205+
let parsed: LoadManagedTableResponse = serde_json::from_str(&resp_body).unwrap();
2206+
assert_eq!(parsed.row_count, 42);
2207+
assert_eq!(parsed.table_name, "orders");
2208+
resolve.assert();
2209+
load.assert();
2210+
}
2211+
21002212
#[test]
21012213
fn tables_delete_uses_default_connection_id() {
21022214
let mut server = mockito::Server::new();

‎src/main.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,7 @@ fn main() {
401401
file,
402402
url,
403403
upload_id,
404+
result_id,
404405
}) => databases::tables_load(
405406
&workspace_id,
406407
Some(catalog.as_str()),
@@ -409,6 +410,7 @@ fn main() {
409410
file.as_deref(),
410411
url.as_deref(),
411412
upload_id.as_deref(),
413+
result_id.as_deref(),
412414
),
413415
Some(DatabasesCommands::Tables { database, command }) => match command {
414416
Some(DatabaseTablesCommands::List {
@@ -428,6 +430,7 @@ fn main() {
428430
file,
429431
url,
430432
upload_id,
433+
result_id,
431434
}) => databases::tables_load(
432435
&workspace_id,
433436
db_flag.as_deref().or(database.as_deref()),
@@ -436,6 +439,7 @@ fn main() {
436439
file.as_deref(),
437440
url.as_deref(),
438441
upload_id.as_deref(),
442+
result_id.as_deref(),
439443
),
440444
Some(DatabaseTablesCommands::Delete {
441445
database: db_flag,

0 commit comments

Comments
 (0)