Skip to content

Commit b3fa057

Browse files
committed
fix: follow truncated inline query results to full set
1 parent f8eb267 commit b3fa057

1 file changed

Lines changed: 174 additions & 2 deletions

File tree

src/query.rs

Lines changed: 174 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,37 @@ pub(crate) fn fetch_arrow_result(api: &Api, result_id: &str) -> QueryResponse {
205205
arrow_result_to_query_response(result, result_id.to_owned())
206206
}
207207

208+
/// Resolve an inline (HTTP 200) query response for display.
209+
///
210+
/// A non-truncated response carries the whole result in `rows`, so it's shown
211+
/// as-is. A truncated one (#640) carries only a bounded preview — the full set
212+
/// is persisted under `result_id` — so follow it to the full result via Arrow,
213+
/// the same path the async (202) branch uses. Truncation rides on result *size*
214+
/// while `async_after_ms` gates on *time*, so a fast-completing but large query
215+
/// returns a truncated inline 200; without this follow the CLI would silently
216+
/// print only the preview rows.
217+
///
218+
/// If a truncated response has no `result_id` (persistence could not be
219+
/// initiated — see the SDK's `warning` field), the full result is unfetchable,
220+
/// so fall back to the preview and surface a warning rather than failing.
221+
fn resolve_inline(api: &Api, resp: hotdata::models::QueryResponse) -> QueryResponse {
222+
if !resp.truncated {
223+
return query_response_from_sdk(resp);
224+
}
225+
match resp.result_id.clone().flatten() {
226+
Some(result_id) => fetch_arrow_result(api, &result_id),
227+
None => {
228+
let mut preview = query_response_from_sdk(resp);
229+
let note = "result truncated to a preview; full result unavailable (persistence not initiated)";
230+
preview.warning = Some(match preview.warning {
231+
Some(w) => format!("{w}; {note}"),
232+
None => note.to_string(),
233+
});
234+
preview
235+
}
236+
}
237+
}
238+
208239
pub fn execute(sql: &str, workspace_id: &str, database: Option<&str>, format: &str) {
209240
let api = Api::new(Some(workspace_id));
210241

@@ -223,9 +254,11 @@ pub fn execute(sql: &str, workspace_id: &str, database: Option<&str>, format: &s
223254
spinner.finish_and_clear();
224255

225256
let async_resp = match outcome {
226-
// Completed within async_after_ms — inline results.
257+
// Completed within async_after_ms — inline results. A large result can
258+
// come back truncated to a preview even on this fast path, so follow it
259+
// to the full set (resolve_inline) rather than printing the preview.
227260
hotdata::QueryOutcome::Inline(resp) => {
228-
print_result(&query_response_from_sdk(resp), format);
261+
print_result(&resolve_inline(&api, resp), format);
229262
return;
230263
}
231264
// Still running — poll the query run, then fetch the result as Arrow.
@@ -397,3 +430,142 @@ pub fn print_result(result: &QueryResponse, format: &str) {
397430
_ => unreachable!(),
398431
}
399432
}
433+
434+
#[cfg(test)]
435+
mod tests {
436+
use super::*;
437+
use crate::sdk::Api;
438+
use std::sync::Arc;
439+
440+
/// A truncated inline 200: one preview row standing in for a larger result.
441+
/// `result_id` uses the wire double-option (`Some(None)` = field present but
442+
/// null, i.e. persistence not initiated).
443+
fn truncated_preview(result_id: Option<&str>) -> hotdata::models::QueryResponse {
444+
let mut resp = hotdata::models::QueryResponse::new(
445+
vec!["id".to_string()], // columns
446+
5, // execution_time_ms
447+
vec![false], // nullable
448+
1, // preview_row_count
449+
"qrun_1".to_string(), // query_run_id
450+
1, // row_count (deprecated, == preview)
451+
vec![vec![serde_json::json!(1)]], // rows (preview only)
452+
true, // truncated
453+
);
454+
resp.result_id = Some(result_id.map(|s| s.to_string()));
455+
resp
456+
}
457+
458+
#[test]
459+
fn resolve_inline_follows_truncated_result_to_full_arrow() {
460+
use arrow::array::{Int64Array, RecordBatch};
461+
use arrow::datatypes::{DataType, Field, Schema};
462+
use arrow::ipc::writer::StreamWriter;
463+
464+
// Full result has 3 rows — more than the 1-row inline preview.
465+
let schema = Arc::new(Schema::new(vec![Field::new("id", DataType::Int64, false)]));
466+
let batch = RecordBatch::try_new(
467+
schema.clone(),
468+
vec![Arc::new(Int64Array::from(vec![1, 2, 3]))],
469+
)
470+
.unwrap();
471+
let mut ipc: Vec<u8> = Vec::new();
472+
{
473+
let mut writer = StreamWriter::try_new(&mut ipc, &schema).unwrap();
474+
writer.write(&batch).unwrap();
475+
writer.finish().unwrap();
476+
}
477+
478+
let mut server = mockito::Server::new();
479+
let m = server
480+
.mock("GET", "/v1/results/res_1")
481+
.match_query(mockito::Matcher::UrlEncoded(
482+
"format".into(),
483+
"arrow".into(),
484+
))
485+
.with_status(200)
486+
.with_header("content-type", "application/vnd.apache.arrow.stream")
487+
.with_body(ipc)
488+
.create();
489+
490+
let api = Api::test_new(&server.url(), "test-jwt", Some("ws-1"));
491+
let resolved = resolve_inline(&api, truncated_preview(Some("res_1")));
492+
493+
// Followed the truncated preview to the full 3-row result.
494+
assert_eq!(resolved.row_count, 3);
495+
assert_eq!(resolved.rows.len(), 3);
496+
assert_eq!(resolved.result_id.as_deref(), Some("res_1"));
497+
m.assert();
498+
}
499+
500+
#[test]
501+
fn resolve_inline_returns_untruncated_preview_without_fetching() {
502+
// truncated=false short-circuits before any network call; point the Api
503+
// at a server with no mocks so an erroneous fetch would fail loudly.
504+
let server = mockito::Server::new();
505+
let api = Api::test_new(&server.url(), "test-jwt", Some("ws-1"));
506+
507+
let mut resp = hotdata::models::QueryResponse::new(
508+
vec!["x".to_string()],
509+
5,
510+
vec![false],
511+
2,
512+
"qrun_2".to_string(),
513+
2,
514+
vec![vec![serde_json::json!(1)], vec![serde_json::json!(2)]],
515+
false, // not truncated
516+
);
517+
resp.result_id = Some(Some("res_2".to_string()));
518+
519+
let resolved = resolve_inline(&api, resp);
520+
assert_eq!(resolved.row_count, 2);
521+
assert_eq!(
522+
resolved.rows,
523+
vec![vec![serde_json::json!(1)], vec![serde_json::json!(2)]]
524+
);
525+
assert_eq!(resolved.result_id.as_deref(), Some("res_2"));
526+
}
527+
528+
#[test]
529+
fn resolve_inline_truncated_without_result_id_warns_and_keeps_preview() {
530+
// Truncated but persistence never started (result_id is null): the full
531+
// result is unfetchable, so keep the preview and surface a warning.
532+
let server = mockito::Server::new();
533+
let api = Api::test_new(&server.url(), "test-jwt", Some("ws-1"));
534+
535+
let resolved = resolve_inline(&api, truncated_preview(None));
536+
assert_eq!(resolved.row_count, 1);
537+
assert_eq!(resolved.rows.len(), 1);
538+
assert!(
539+
resolved
540+
.warning
541+
.as_deref()
542+
.unwrap_or("")
543+
.contains("truncated")
544+
);
545+
}
546+
547+
#[test]
548+
fn resolve_inline_preserves_existing_warning_when_following_fails() {
549+
// A truncated response with no result_id often arrives with an SDK
550+
// warning explaining why persistence didn't start. The truncation note
551+
// is appended to it, not allowed to clobber it.
552+
let server = mockito::Server::new();
553+
let api = Api::test_new(&server.url(), "test-jwt", Some("ws-1"));
554+
555+
let mut resp = truncated_preview(None);
556+
resp.warning = Some(Some(
557+
"result persistence could not be initiated".to_string(),
558+
));
559+
560+
let resolved = resolve_inline(&api, resp);
561+
let warning = resolved.warning.as_deref().unwrap_or("");
562+
assert!(
563+
warning.contains("result persistence could not be initiated"),
564+
"original warning dropped: {warning:?}"
565+
);
566+
assert!(
567+
warning.contains("truncated"),
568+
"truncation note missing: {warning:?}"
569+
);
570+
}
571+
}

0 commit comments

Comments
 (0)