Skip to content

Commit a7e9d82

Browse files
committed
fix(query): trust the server's next-page link for the table cap
The probe row only proves there is more when `?limit=` is honoured exactly. A server free to clamp the limit to its own maximum returns the window full and no further row, so the probe never arrives, and a window with no X-Total-Row-Count printed as a complete table again — the same silent truncation through a different door. `next_link` is the server stating there is another page, and it was already being constructed and dropped. It now counts alongside the probe row, so either signal alone is enough to mark the window a preview.
1 parent 98c1a5d commit a7e9d82

1 file changed

Lines changed: 45 additions & 2 deletions

File tree

‎src/commands/query.rs‎

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,11 @@ fn fetch_capped(api: &Api, meta: &StreamMeta, cap: i64) -> Result<QueryResponse,
832832
while let Some(batch) = api.next_result_batch(&mut stream)? {
833833
batches.push(batch);
834834
}
835+
// The server's own "there is more" signal. Independent of the probe row,
836+
// which only fires when `?limit=` was honoured exactly: a server free to
837+
// clamp the limit to its own maximum returns the window full and no more,
838+
// and without this that would read as a complete result all over again.
839+
let more_pages = stream.next_link().is_some();
835840
let result = hotdata::ArrowResult {
836841
batches,
837842
schema,
@@ -850,8 +855,9 @@ fn fetch_capped(api: &Api, meta: &StreamMeta, cap: i64) -> Result<QueryResponse,
850855
response.rows.truncate(cap_rows as usize);
851856
response.row_count = cap_rows;
852857
}
853-
response.truncated =
854-
beyond_window || reported_total.is_some_and(|total| total > response.row_count);
858+
response.truncated = beyond_window
859+
|| more_pages
860+
|| reported_total.is_some_and(|total| total > response.row_count);
855861
// `?` in the footer beats a total derived from the window, which would read
856862
// as "10000 of 10000" for a result of a million.
857863
response.total_row_count = reported_total;
@@ -2217,6 +2223,43 @@ mod tests {
22172223
assert_eq!(result.execution_time_ms, Some(4200));
22182224
}
22192225

2226+
/// A server that clamps `?limit=` to its own maximum returns the window
2227+
/// exactly full, so the probe row never arrives and cannot prove anything.
2228+
/// The `Link: rel="next"` header is the server saying there is more, and it
2229+
/// is what keeps that case from printing as a complete result.
2230+
#[test]
2231+
fn a_next_link_marks_a_window_incomplete_when_the_probe_row_never_arrives() {
2232+
let mut server = mockito::Server::new();
2233+
// Exactly the window, no total header — but a next link.
2234+
let _m = server
2235+
.mock("GET", "/v1/results/res_1")
2236+
.match_query(mockito::Matcher::UrlEncoded(
2237+
"format".into(),
2238+
"arrow".into(),
2239+
))
2240+
.with_status(200)
2241+
.with_header("content-type", "application/vnd.apache.arrow.stream")
2242+
.with_header(
2243+
"Link",
2244+
"<https://api.hotdata.dev/v1/results/res_1?offset=6>; rel=\"next\"",
2245+
)
2246+
.with_body(awkward_ipc())
2247+
.create();
2248+
let api = Api::test_new_scoped(&server.url(), "test-jwt", Some("ws-1"), Some("db-1"));
2249+
2250+
// cap 6 == the rows served, so `beyond_window` is false and only the
2251+
// link can carry the fact.
2252+
let result = fetch_capped(&api, &meta_for("res_1"), 6).unwrap();
2253+
2254+
assert_eq!(result.row_count, 6, "the whole window is shown");
2255+
assert!(
2256+
result.truncated,
2257+
"the server said there is a next page; a full window with no total \
2258+
header must not print as a complete result"
2259+
);
2260+
assert_eq!(result_exit_code(&result), EXIT_INCOMPLETE_RESULT);
2261+
}
2262+
22202263
/// The cap must not depend on the server reporting a row count.
22212264
///
22222265
/// `arrow_result_to_query_response` backfills `total_row_count` from the

0 commit comments

Comments
 (0)