Skip to content

Commit 608b8b2

Browse files
Pagination
1 parent 76b7369 commit 608b8b2

File tree

7 files changed

+232
-37
lines changed

7 files changed

+232
-37
lines changed

src/tui/execution.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,31 @@ impl TuiExecution {
403403
}
404404
}
405405

406+
#[cfg(feature = "flightsql")]
407+
pub async fn next_flightsql_batch(&self, sql: String, sender: UnboundedSender<AppEvent>) {
408+
let mut streams = self.flightsql_result_stream.lock().await;
409+
if let Some(s) = streams.as_mut() {
410+
let start = std::time::Instant::now();
411+
if let Some((ticket, batch_result)) = s.next().await {
412+
match batch_result {
413+
Ok(batch) => {
414+
info!("Fetched next FlightSQL batch from {}: {} rows", ticket, batch.num_rows());
415+
let duration = start.elapsed();
416+
let results = ExecutionResultsBatch {
417+
query: sql,
418+
batch,
419+
duration,
420+
};
421+
let _ = sender.send(AppEvent::FlightSQLExecutionResultsNextBatch(results));
422+
}
423+
Err(e) => {
424+
error!("Error getting next FlightSQL batch: {:?}", e);
425+
}
426+
}
427+
}
428+
}
429+
}
430+
406431
// TODO: Maybe just expose `inner` and use that rather than re-implementing the same
407432
// functions here.
408433
#[cfg(feature = "flightsql")]

src/tui/handlers/flightsql.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,26 @@ pub fn normal_mode_handler(app: &mut App, key: KeyEvent) {
7878
}
7979
(KeyCode::Right, KeyModifiers::NONE) => {
8080
let _event_tx = app.event_tx();
81+
82+
if let Some(current_page) = app.state.flightsql_tab.current_page() {
83+
let next_page = current_page + 1;
84+
85+
// Check if we need more batches for the next page
86+
if app.state.flightsql_tab.needs_more_batches_for_page(next_page) {
87+
info!("Fetching more batches for page {}", next_page);
88+
89+
if let Some(last_query) = app.state.history_tab.history().last() {
90+
let execution = Arc::clone(&app.execution);
91+
let sql = last_query.sql().clone();
92+
tokio::spawn(async move {
93+
execution.next_flightsql_batch(sql, _event_tx).await;
94+
});
95+
}
96+
return; // Wait for batch to load before advancing page
97+
}
98+
}
99+
100+
// Sufficient data available, advance page
81101
if let Err(e) = _event_tx.send(AppEvent::FlightSQLExecutionResultsNextPage) {
82102
error!("Error going to next FlightSQL results page: {e}");
83103
}

src/tui/handlers/mod.rs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -237,15 +237,23 @@ pub fn app_event_handler(app: &mut App, event: AppEvent) -> Result<()> {
237237
duration,
238238
batch,
239239
} = r;
240-
info!("Adding batch to flightsql tab");
240+
241+
let is_first_batch = app.state.flightsql_tab.current_page().is_none();
242+
241243
app.state.flightsql_tab.set_in_progress(false);
242244
app.state.flightsql_tab.add_batch(batch);
243-
app.state.flightsql_tab.next_page();
244-
app.state.flightsql_tab.refresh_query_results_state();
245-
let history_query =
246-
HistoryQuery::new(Context::FlightSQL, query.to_string(), duration, None, None);
247-
app.state.history_tab.add_to_history(history_query);
248-
app.state.history_tab.refresh_history_table_state();
245+
246+
if is_first_batch {
247+
app.state.flightsql_tab.next_page();
248+
app.state.flightsql_tab.refresh_query_results_state();
249+
250+
let history_query =
251+
HistoryQuery::new(Context::FlightSQL, query.to_string(), duration, None, None);
252+
app.state.history_tab.add_to_history(history_query);
253+
app.state.history_tab.refresh_history_table_state();
254+
} else {
255+
app.state.flightsql_tab.refresh_query_results_state();
256+
}
249257
}
250258
#[cfg(feature = "flightsql")]
251259
AppEvent::FlightSQLEstablishConnection => {

src/tui/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,12 @@
1717

1818
pub mod execution;
1919
pub mod handlers;
20+
mod pagination;
2021
pub mod state;
2122
pub mod ui;
2223

24+
pub use pagination::{extract_page, has_sufficient_rows, page_row_range, PAGE_SIZE};
25+
2326
use color_eyre::eyre::eyre;
2427
use color_eyre::Result;
2528
use datafusion_app::config::merge_configs;

src/tui/pagination.rs

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
use datafusion::arrow::array::{Array, RecordBatch, UInt32Array};
2+
use datafusion::arrow::compute::{concat_batches, take_record_batch};
3+
use datafusion::arrow::error::ArrowError;
4+
use std::sync::Arc;
5+
6+
pub const PAGE_SIZE: usize = 100;
7+
8+
/// Calculate the row range needed for a given page
9+
pub fn page_row_range(page: usize, page_size: usize) -> (usize, usize) {
10+
let start = page * page_size;
11+
let end = start + page_size;
12+
(start, end)
13+
}
14+
15+
/// Check if we have enough rows loaded to display the requested page
16+
pub fn has_sufficient_rows(loaded_rows: usize, page: usize, page_size: usize) -> bool {
17+
let (_start, end) = page_row_range(page, page_size);
18+
loaded_rows >= end
19+
}
20+
21+
/// Extract a page of rows from loaded batches
22+
/// This handles pagination across batch boundaries by concatenating only what's needed
23+
pub fn extract_page(
24+
batches: &[RecordBatch],
25+
page: usize,
26+
page_size: usize,
27+
) -> Result<RecordBatch, ArrowError> {
28+
if batches.is_empty() {
29+
return Ok(RecordBatch::new_empty(Arc::new(
30+
datafusion::arrow::datatypes::Schema::empty(),
31+
)));
32+
}
33+
34+
let total_rows: usize = batches.iter().map(|b| b.num_rows()).sum();
35+
let (start, end) = page_row_range(page, page_size);
36+
37+
// Clamp end to available rows
38+
let end = end.min(total_rows);
39+
40+
if start >= total_rows {
41+
// Page is beyond available data
42+
return Ok(RecordBatch::new_empty(batches[0].schema()));
43+
}
44+
45+
// Create indices for the rows we want
46+
let indices = UInt32Array::from_iter_values((start as u32)..(end as u32));
47+
48+
// Extract rows from batches
49+
extract_rows_from_batches(batches, &indices)
50+
}
51+
52+
/// Extract specific rows (by global indices) from batches
53+
/// Handles batch boundaries by concatenating only necessary batches
54+
fn extract_rows_from_batches(
55+
batches: &[RecordBatch],
56+
indices: &dyn Array,
57+
) -> Result<RecordBatch, ArrowError> {
58+
match batches.len() {
59+
0 => Ok(RecordBatch::new_empty(Arc::new(
60+
datafusion::arrow::datatypes::Schema::empty(),
61+
))),
62+
1 => take_record_batch(&batches[0], indices),
63+
_ => {
64+
// Multiple batches: concat then extract rows
65+
// Only concat the batches we've loaded (lazy loading ensures minimal concat)
66+
let schema = batches[0].schema();
67+
let concatenated = concat_batches(&schema, batches)?;
68+
take_record_batch(&concatenated, indices)
69+
}
70+
}
71+
}
72+
73+
#[cfg(test)]
74+
mod tests {
75+
use super::*;
76+
77+
#[test]
78+
fn test_page_row_range() {
79+
assert_eq!(page_row_range(0, 100), (0, 100));
80+
assert_eq!(page_row_range(1, 100), (100, 200));
81+
assert_eq!(page_row_range(2, 50), (100, 150));
82+
}
83+
84+
#[test]
85+
fn test_has_sufficient_rows() {
86+
assert!(has_sufficient_rows(100, 0, 100)); // Exactly enough
87+
assert!(has_sufficient_rows(150, 0, 100)); // More than enough
88+
assert!(!has_sufficient_rows(50, 0, 100)); // Not enough
89+
assert!(!has_sufficient_rows(150, 1, 100)); // Need 200, only have 150
90+
}
91+
}

src/tui/state/tabs/flightsql.rs

Lines changed: 15 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,8 @@ use std::sync::Arc;
2020

2121
use color_eyre::Result;
2222
use datafusion::arrow::{
23-
array::{Array, RecordBatch, UInt32Array},
24-
compute::take_record_batch,
23+
array::RecordBatch,
2524
datatypes::Schema,
26-
error::ArrowError,
2725
};
2826
use log::{error, info};
2927
use ratatui::crossterm::event::KeyEvent;
@@ -34,11 +32,10 @@ use tokio::task::JoinHandle;
3432
use tui_textarea::TextArea;
3533

3634
use crate::config::AppConfig;
35+
use crate::tui::pagination::{extract_page, has_sufficient_rows, PAGE_SIZE};
3736
use crate::tui::state::tabs::sql;
3837
use crate::tui::ExecutionError;
3938

40-
const PAGE_SIZE: usize = 100;
41-
4239
#[derive(Debug, Default)]
4340
pub enum FlightSQLConnectionStatus {
4441
#[default]
@@ -217,22 +214,10 @@ impl FlightSQLTabState<'_> {
217214
pub fn current_page_results(&self) -> Option<RecordBatch> {
218215
match (self.current_page, self.result_batches.as_ref()) {
219216
(Some(page), Some(batches)) => {
220-
let total_rows: usize = batches.iter().map(|b| b.num_rows()).sum();
221-
let indices = if total_rows < PAGE_SIZE {
222-
UInt32Array::from_iter_values(0_u32..(total_rows as u32))
223-
} else {
224-
let start = page * PAGE_SIZE;
225-
let remaining = total_rows - start;
226-
// On the last page there could be less than PAGE_SIZE results to view
227-
let page_records = remaining.min(PAGE_SIZE);
228-
let end = (start as u32) + (page_records as u32);
229-
info!("Current page start({start}) end({end})");
230-
UInt32Array::from_iter_values((start as u32)..end)
231-
};
232-
match take_record_batches(batches, &indices) {
217+
match extract_page(batches, page, PAGE_SIZE) {
233218
Ok(batch) => Some(batch),
234219
Err(err) => {
235-
error!("Error getting record batch: {}", err);
220+
error!("Error getting page {}: {}", page, err);
236221
None
237222
}
238223
}
@@ -241,6 +226,17 @@ impl FlightSQLTabState<'_> {
241226
}
242227
}
243228

229+
pub fn total_loaded_rows(&self) -> usize {
230+
self.result_batches
231+
.as_ref()
232+
.map(|batches| batches.iter().map(|b| b.num_rows()).sum())
233+
.unwrap_or(0)
234+
}
235+
236+
pub fn needs_more_batches_for_page(&self, page: usize) -> bool {
237+
!has_sufficient_rows(self.total_loaded_rows(), page, PAGE_SIZE)
238+
}
239+
244240
pub fn next_page(&mut self) {
245241
self.change_page(
246242
|page, max_pages| {
@@ -309,14 +305,3 @@ impl FlightSQLTabState<'_> {
309305
}
310306
}
311307

312-
fn take_record_batches(
313-
batches: &[RecordBatch],
314-
indices: &dyn Array,
315-
) -> Result<RecordBatch, ArrowError> {
316-
match batches.len() {
317-
0 => Ok(RecordBatch::new_empty(Arc::new(Schema::empty()))),
318-
1 => take_record_batch(&batches[0], indices),
319-
// For now we just get the first batch
320-
_ => take_record_batch(&batches[0], indices),
321-
}
322-
}

tests/tui_cases/flightsql_pagination.rs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ fn create_values_query(num: usize) -> String {
8383
format!("{base} {vals}")
8484
}
8585

86+
fn create_values_query_offset(num: usize, offset: usize) -> String {
87+
let base = "SELECT * FROM VALUES";
88+
let vals = (offset..offset + num).map(|i| format!("({i})")).join(",");
89+
format!("{base} {vals}")
90+
}
91+
8692
// Tests that we can paginate through multiple pages and go back to the first page
8793
#[tokio::test]
8894
async fn multiple_pages_forward_and_back() {
@@ -199,3 +205,60 @@ async fn multiple_pages_forward_and_back_and_forward() {
199205
assert_batches_eq!(expected, &batches);
200206
}
201207
}
208+
209+
// Tests lazy loading: only load batches as needed for pagination
210+
// Simulates 3 batches: 60 rows, 60 rows, 20 rows (140 total)
211+
#[tokio::test]
212+
async fn multiple_batches_lazy_loading() {
213+
let mut test_app = TestApp::new().await;
214+
215+
test_app
216+
.handle_app_event(AppEvent::FlightSQLNewExecution)
217+
.unwrap();
218+
219+
// Send only first batch initially (lazy loading)
220+
let batch1 = create_execution_results(&create_values_query(60)).await;
221+
test_app
222+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextBatch(batch1))
223+
.unwrap();
224+
225+
// Verify page 0 shows 60 rows (only first batch loaded)
226+
{
227+
let state = test_app.state();
228+
assert_eq!(state.flightsql_tab.current_page().unwrap(), 0);
229+
let page_results = state.flightsql_tab.current_page_results().unwrap();
230+
assert_eq!(page_results.num_rows(), 60);
231+
}
232+
233+
// Send second batch (simulating lazy load)
234+
let batch2 = create_execution_results(&create_values_query_offset(60, 60)).await;
235+
test_app
236+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextBatch(batch2))
237+
.unwrap();
238+
239+
// Now page 0 should show 100 rows (spanning both batches)
240+
{
241+
let state = test_app.state();
242+
let page_results = state.flightsql_tab.current_page_results().unwrap();
243+
assert_eq!(page_results.num_rows(), 100);
244+
}
245+
246+
// Go to page 1
247+
test_app
248+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextPage)
249+
.unwrap();
250+
251+
// Send third batch
252+
let batch3 = create_execution_results(&create_values_query_offset(20, 120)).await;
253+
test_app
254+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextBatch(batch3))
255+
.unwrap();
256+
257+
// Verify page 1 shows remaining rows
258+
{
259+
let state = test_app.state();
260+
assert_eq!(state.flightsql_tab.current_page().unwrap(), 1);
261+
let page_results = state.flightsql_tab.current_page_results().unwrap();
262+
assert_eq!(page_results.num_rows(), 40);
263+
}
264+
}

0 commit comments

Comments
 (0)