Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions src/commands/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,13 @@ struct QueryRun {
execution_time_ms: Option<u64>,
server_processing_ms: Option<u64>,
row_count: Option<u64>,
/// Bytes of table data read from storage to run this query. `None` when the
/// query touches no table (e.g. `SELECT 1`); may be `0` for a count served
/// from table statistics.
bytes_scanned: Option<u64>,
/// Rows read from storage before filtering/aggregation, distinct from
/// `row_count` (rows returned). `None` when no table data was read.
rows_scanned: Option<u64>,
saved_query_id: Option<String>,
saved_query_version: Option<u64>,
snapshot_id: String,
Expand All @@ -166,6 +173,8 @@ impl From<QueryRunInfo> for QueryRun {
execution_time_ms: r.execution_time_ms.flatten().map(|v| v.max(0) as u64),
server_processing_ms: r.server_processing_ms.flatten().map(|v| v.max(0) as u64),
row_count: r.row_count.flatten().map(|v| v.max(0) as u64),
bytes_scanned: r.bytes_scanned.flatten().map(|v| v.max(0) as u64),
rows_scanned: r.rows_scanned.flatten().map(|v| v.max(0) as u64),
saved_query_id: r.saved_query_id.flatten(),
saved_query_version: r.saved_query_version.flatten().map(|v| v.max(0) as u64),
snapshot_id: r.snapshot_id,
Expand Down Expand Up @@ -301,6 +310,12 @@ fn print_detail(r: &QueryRun, format: &str) {
if let Some(n) = r.row_count {
println!("{}{}", label("rows:"), n);
}
if let Some(n) = r.rows_scanned {
println!("{}{}", label("rows scanned:"), n);
}
if let Some(n) = r.bytes_scanned {
println!("{}{}", label("bytes scanned:"), crate::util::human_bytes(n));
}
if let Some(ref id) = r.result_id {
println!("{}{}", label("result id:"), id);
}
Expand Down Expand Up @@ -345,3 +360,49 @@ fn print_detail(r: &QueryRun, format: &str) {
_ => unreachable!(),
}
}

#[cfg(test)]
mod tests {
use super::*;

/// A real `GET /v1/query-runs/{id}` body carries `bytes_scanned` and
/// `rows_scanned`; both must survive the `QueryRunInfo` → `QueryRun` mapping
/// and appear in the serialized (JSON/YAML) output.
#[test]
fn maps_scanned_fields_from_wire_body() {
let body = r#"{
"id":"qrun_1","status":"succeeded",
"created_at":"2026-07-20T00:00:00Z",
"snapshot_id":"snap_1","sql_hash":"h","sql_text":"SELECT 1",
"row_count":42,"rows_scanned":1000,"bytes_scanned":98209424
}"#;
let info: QueryRunInfo = serde_json::from_str(body).unwrap();
let run: QueryRun = info.into();

assert_eq!(run.row_count, Some(42));
assert_eq!(run.rows_scanned, Some(1000));
assert_eq!(run.bytes_scanned, Some(98_209_424));

let json = serde_json::to_value(&run).unwrap();
assert_eq!(json["bytes_scanned"], 98_209_424);
assert_eq!(json["rows_scanned"], 1000);
}

/// A constant-expression query (e.g. `SELECT 1`) reads no table, so the API
/// omits the scanned fields; they map to `None` and serialize as null.
#[test]
fn absent_scanned_fields_map_to_none() {
let info = QueryRunInfo::new(
"2026-07-20T00:00:00Z".to_string(),
"qrun_2".to_string(),
"snap_1".to_string(),
"h".to_string(),
"SELECT 1".to_string(),
"succeeded".to_string(),
);
let run: QueryRun = info.into();

assert_eq!(run.bytes_scanned, None);
assert_eq!(run.rows_scanned, None);
}
}
128 changes: 87 additions & 41 deletions src/commands/query.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::client::sdk::{Api, ApiError};
use serde::Deserialize;
use serde::Serialize;
use serde_json::Value;

/// Subcommands for `hotdata query`.
Expand All @@ -14,8 +14,13 @@ pub enum QueryCommands {
},
}

#[derive(Deserialize)]
#[derive(Serialize)]
pub struct QueryResponse {
/// ID of the query run that produced this result. Surfaced so a user can
/// look up run-level metadata (e.g. bytes/rows scanned) with
/// `hotdata queries <id>`. `None` on paths that fetch a persisted result by
/// `result_id` alone and never learn the originating run.
pub query_run_id: Option<String>,
pub result_id: Option<String>,
pub columns: Vec<String>,
pub rows: Vec<Vec<Value>>,
Expand All @@ -33,6 +38,10 @@ pub struct QueryResponse {
/// whole result. Drives the fail-closed exit in [`print_result`].
pub truncated: bool,
pub execution_time_ms: Option<u64>,
/// A human-facing completeness notice (e.g. truncation). Printed to stderr by
/// [`print_result`], never the stdout body — a JSON consumer reads the same
/// fact from the typed `truncated` / `total_row_count` fields.
#[serde(skip)]
pub warning: Option<String>,
}

Expand Down Expand Up @@ -73,6 +82,7 @@ fn query_response_from_sdk(resp: hotdata::models::QueryResponse) -> QueryRespons
Some(row_count)
});
QueryResponse {
query_run_id: (!resp.query_run_id.is_empty()).then_some(resp.query_run_id),
result_id: resp.result_id.flatten(),
columns: resp.columns,
row_count,
Expand Down Expand Up @@ -252,6 +262,9 @@ fn arrow_result_to_query_response(
.and_then(|t| u64::try_from(t).ok())
.or(Some(row_count));
QueryResponse {
// The Arrow fetch is keyed by result_id and carries no run id; the
// async/poll callers that know it stamp it after this returns.
query_run_id: None,
result_id: Some(result_id),
columns,
rows,
Expand Down Expand Up @@ -332,11 +345,12 @@ fn resolve_inline(api: &Api, resp: hotdata::models::QueryResponse) -> QueryRespo
match resp.result_id.clone().flatten() {
Some(result_id) => match try_fetch_arrow_result(api, &result_id) {
// The Arrow fetch returns only schema + rows; carry the query-level
// warning and execution time the inline response reported, which
// `arrow_result_to_query_response` otherwise hardcodes to None.
// warning, execution time, and run id the inline response reported,
// which `arrow_result_to_query_response` otherwise hardcodes to None.
Ok(mut full) => {
full.warning = resp.warning.flatten();
full.execution_time_ms = Some(resp.execution_time_ms.max(0) as u64);
full.query_run_id = (!resp.query_run_id.is_empty()).then_some(resp.query_run_id);
full
}
// The full result is persisted but the follow-up fetch failed (e.g.
Expand Down Expand Up @@ -483,8 +497,9 @@ pub fn execute(sql: &str, workspace_id: &str, database: Option<&str>, format: &s
let execution_time_ms = run.execution_time_ms;
match run.result_id.flatten() {
Some(ref result_id) => {
let result =
let mut result =
fetch_arrow_result_with_timing(&api, result_id, execution_time_ms);
result.query_run_id = Some(run_id.clone());
print_result(&result, format);
}
None => {
Expand Down Expand Up @@ -544,7 +559,9 @@ pub fn poll(query_run_id: &str, workspace_id: &str, database: Option<&str>, form
let execution_time_ms = run.execution_time_ms;
match run.result_id.flatten() {
Some(ref result_id) => {
let result = fetch_arrow_result_with_timing(&api, result_id, execution_time_ms);
let mut result =
fetch_arrow_result_with_timing(&api, result_id, execution_time_ms);
result.query_run_id = Some(run.id.clone());
print_result(&result, format);
}
None => {
Expand Down Expand Up @@ -573,26 +590,6 @@ pub fn poll(query_run_id: &str, workspace_id: &str, database: Option<&str>, form
}
}

/// Build the `--output json` body for a result.
///
/// `row_count` (rows in this body) is kept for backward compatibility, but the
/// truncation truth is carried explicitly: `truncated`, `preview_row_count`
/// (rows held), and `total_row_count` (grand total, `null` when unknown). A
/// JSON consumer can detect a partial result from these rather than having to
/// notice a stderr-only warning.
fn result_json(result: &QueryResponse) -> Value {
serde_json::json!({
"result_id": result.result_id,
"columns": result.columns,
"rows": result.rows,
"row_count": result.row_count,
"preview_row_count": result.row_count,
"total_row_count": result.total_row_count,
"truncated": result.truncated,
"execution_time_ms": result.execution_time_ms,
})
}

/// Process exit code after rendering a result: [`EXIT_INCOMPLETE_RESULT`] when
/// the rows are an incomplete preview (fail closed so pipelines break), else `0`.
fn result_exit_code(result: &QueryResponse) -> i32 {
Expand All @@ -609,6 +606,11 @@ fn result_exit_code(result: &QueryResponse) -> i32 {
/// loud — `N of TOTAL rows — INCOMPLETE PREVIEW (...)` — with `?` standing in for
/// a total the server didn't report. The caller colours it (red vs grey).
fn table_footer(result: &QueryResponse) -> String {
let run_part = result
.query_run_id
.as_deref()
.map(|id| format!(" [run: {id}]"))
.unwrap_or_default();
let id_part = result
.result_id
.as_deref()
Expand All @@ -624,15 +626,16 @@ fn table_footer(result: &QueryResponse) -> String {
.map(|t| t.to_string())
.unwrap_or_else(|| "?".to_string());
format!(
"{} of {} rows — INCOMPLETE PREVIEW ({}){}",
result.row_count, total, time_part, id_part
"{} of {} rows — INCOMPLETE PREVIEW ({}){}{}",
result.row_count, total, time_part, run_part, id_part
)
} else {
format!(
"{} row{} ({}){}",
"{} row{} ({}){}{}",
result.row_count,
if result.row_count == 1 { "" } else { "s" },
time_part,
run_part,
id_part
)
}
Expand All @@ -645,10 +648,9 @@ pub fn print_result(result: &QueryResponse, format: &str) {

match format {
"json" => {
println!(
"{}",
serde_json::to_string_pretty(&result_json(result)).unwrap()
);
// Serialize the display struct directly; `warning` is `#[serde(skip)]`
// (stderr-only), the rest is the JSON body.
println!("{}", serde_json::to_string_pretty(result).unwrap());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: (not blocking) Serializing the struct directly drops the preview_row_count field the old result_json emitted, so this is a backward-incompatible change to the --output json shape. The value is still recoverable from row_count (they were always equal), but any consumer reading preview_row_count will now get undefined. Worth confirming this is intended and noting it in the changelog / release notes if the JSON output is a public contract.

}
"csv" => {
println!("{}", result.columns.join(","));
Expand Down Expand Up @@ -800,6 +802,9 @@ mod tests {
// Inline warning + timing carried through, not dropped by the fetch.
assert_eq!(resolved.warning.as_deref(), Some("approximate aggregate"));
assert_eq!(resolved.execution_time_ms, Some(5));
// The run id from the inline response must survive the Arrow follow — the
// Arrow path itself has no run id, so the follow branch must stamp it.
assert_eq!(resolved.query_run_id.as_deref(), Some("qrun_1"));
m.assert();
}

Expand Down Expand Up @@ -897,11 +902,12 @@ mod tests {
}

#[test]
fn result_json_exposes_truncation_for_incomplete_preview() {
fn json_body_exposes_truncation_for_incomplete_preview() {
// A JSON consumer must be able to detect a partial result from the body
// alone — not only from a stderr warning. truncated=true, preview <
// total, and both counts present.
// alone — not only from a stderr warning. truncated=true, row_count <
// total, both present. The stderr-only `warning` must NOT leak in.
let result = QueryResponse {
query_run_id: None,
result_id: None,
columns: vec!["id".to_string()],
rows: vec![vec![serde_json::json!(1)]],
Expand All @@ -911,16 +917,17 @@ mod tests {
execution_time_ms: Some(5),
warning: Some("result truncated to a preview".to_string()),
};
let json = result_json(&result);
let json = serde_json::to_value(&result).unwrap();
assert_eq!(json["truncated"], serde_json::json!(true));
assert_eq!(json["preview_row_count"], serde_json::json!(1));
assert_eq!(json["row_count"], serde_json::json!(1));
assert_eq!(json["total_row_count"], serde_json::json!(100));
assert!(json.get("warning").is_none(), "warning leaked into JSON");
}

#[test]
fn result_json_marks_complete_result_not_truncated() {
fn json_body_marks_complete_result_not_truncated() {
let result = QueryResponse {
query_run_id: Some("qrun_9".to_string()),
result_id: Some("res_9".to_string()),
columns: vec!["id".to_string()],
rows: vec![vec![serde_json::json!(1)], vec![serde_json::json!(2)]],
Expand All @@ -930,9 +937,9 @@ mod tests {
execution_time_ms: Some(5),
warning: None,
};
let json = result_json(&result);
let json = serde_json::to_value(&result).unwrap();
assert_eq!(json["truncated"], serde_json::json!(false));
assert_eq!(json["preview_row_count"], serde_json::json!(2));
assert_eq!(json["row_count"], serde_json::json!(2));
assert_eq!(json["total_row_count"], serde_json::json!(2));
// A complete result exits 0 — pipelines proceed.
assert_eq!(result_exit_code(&result), 0);
Expand All @@ -944,6 +951,7 @@ mod tests {
// non-generic exit code so a pipeline breaks instead of ingesting a
// partial result that exited 0.
let result = QueryResponse {
query_run_id: None,
result_id: None,
columns: vec!["id".to_string()],
rows: vec![vec![serde_json::json!(1)]],
Expand All @@ -967,6 +975,7 @@ mod tests {
truncated: bool,
) -> QueryResponse {
QueryResponse {
query_run_id: None,
result_id: None,
columns: vec!["id".to_string()],
rows: Vec::new(),
Expand Down Expand Up @@ -1000,6 +1009,43 @@ mod tests {
assert!(footer.starts_with("2 rows"), "footer: {footer}");
}

#[test]
fn table_footer_includes_run_id_when_present() {
// The run id lets a user follow up with `hotdata queries <id>` for
// run-level metadata (bytes/rows scanned). It precedes the result-id.
let mut result = display_result(2, Some(2), false);
result.query_run_id = Some("qrun_7b3e04".to_string());
result.result_id = Some("res_9f2a1c".to_string());
let footer = table_footer(&result);
assert!(footer.contains("[run: qrun_7b3e04]"), "footer: {footer}");
let run_at = footer.find("[run:").unwrap();
let res_at = footer.find("[result-id:").unwrap();
assert!(run_at < res_at, "run id should precede result id: {footer}");
}

#[test]
fn table_footer_omits_run_id_when_absent() {
// The Arrow-only fetch path has no run id; the footer must not render an
// empty `[run: ]` tag.
let footer = table_footer(&display_result(2, Some(2), false));
assert!(!footer.contains("[run:"), "footer: {footer}");
}

#[test]
fn json_body_exposes_query_run_id() {
let mut result = display_result(2, Some(2), false);
result.query_run_id = Some("qrun_7b3e04".to_string());
let json = serde_json::to_value(&result).unwrap();
assert_eq!(json["query_run_id"], serde_json::json!("qrun_7b3e04"));
}

#[test]
fn inline_response_carries_query_run_id_from_sdk() {
// The SDK inline response carries the run id; the CLI must not drop it.
let resolved = query_response_from_sdk(truncated_preview(Some("res_1")));
assert_eq!(resolved.query_run_id.as_deref(), Some("qrun_1"));
}

#[test]
fn resolve_inline_preserves_existing_warning_when_following_fails() {
// A truncated response with no result_id often arrives with an SDK
Expand Down
Loading
Loading