Skip to content

Commit 58827d6

Browse files
Fix
1 parent 608b8b2 commit 58827d6

File tree

2 files changed

+182
-0
lines changed

2 files changed

+182
-0
lines changed

src/tui/handlers/mod.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,27 @@ pub fn app_event_handler(app: &mut App, event: AppEvent) -> Result<()> {
253253
app.state.history_tab.refresh_history_table_state();
254254
} else {
255255
app.state.flightsql_tab.refresh_query_results_state();
256+
257+
// Check if we have enough data for the next page now
258+
// If not, automatically fetch another batch
259+
if let Some(current_page) = app.state.flightsql_tab.current_page() {
260+
let next_page = current_page + 1;
261+
if app.state.flightsql_tab.needs_more_batches_for_page(next_page) {
262+
info!("Still need more batches for page {}, fetching next batch", next_page);
263+
let execution = Arc::clone(&app.execution);
264+
let sql = query.clone();
265+
let _event_tx = app.event_tx();
266+
tokio::spawn(async move {
267+
execution.next_flightsql_batch(sql, _event_tx).await;
268+
});
269+
} else {
270+
// We now have enough data, advance to the page
271+
info!("Sufficient data loaded, advancing to page {}", next_page);
272+
if let Err(e) = app.event_tx().send(AppEvent::FlightSQLExecutionResultsNextPage) {
273+
error!("Error advancing to next page: {e}");
274+
}
275+
}
276+
}
256277
}
257278
}
258279
#[cfg(feature = "flightsql")]

tests/tui_cases/flightsql_pagination.rs

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,3 +262,164 @@ async fn multiple_batches_lazy_loading() {
262262
assert_eq!(page_results.num_rows(), 40);
263263
}
264264
}
265+
266+
// Tests that multiple small batches are automatically loaded to fill a page
267+
// This verifies the fix for the issue where user would have to press Right multiple times
268+
// Scenario: Page needs 100 rows, but each batch only has 30 rows
269+
#[tokio::test]
270+
async fn multiple_small_batches_auto_load() {
271+
let mut test_app = TestApp::new().await;
272+
273+
test_app
274+
.handle_app_event(AppEvent::FlightSQLNewExecution)
275+
.unwrap();
276+
277+
// Send first batch: 100 rows (fills page 0)
278+
let batch1 = create_execution_results(&create_values_query(100)).await;
279+
test_app
280+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextBatch(batch1))
281+
.unwrap();
282+
283+
// Verify we're on page 0 with 100 rows
284+
{
285+
let state = test_app.state();
286+
assert_eq!(state.flightsql_tab.current_page().unwrap(), 0);
287+
assert_eq!(state.flightsql_tab.total_loaded_rows(), 100);
288+
}
289+
290+
// Send second batch: only 30 rows (NOT enough for page 1 which needs rows 100-199)
291+
// The system should NOT advance the page yet
292+
let batch2 = create_execution_results(&create_values_query_offset(30, 100)).await;
293+
test_app
294+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextBatch(batch2))
295+
.unwrap();
296+
297+
// Verify we're still on page 0, but now have 130 rows total
298+
{
299+
let state = test_app.state();
300+
assert_eq!(state.flightsql_tab.current_page().unwrap(), 0);
301+
assert_eq!(state.flightsql_tab.total_loaded_rows(), 130);
302+
// Verify we still need more batches for page 1
303+
assert!(state.flightsql_tab.needs_more_batches_for_page(1));
304+
}
305+
306+
// Send third batch: another 30 rows (total 160, still NOT enough)
307+
let batch3 = create_execution_results(&create_values_query_offset(30, 130)).await;
308+
test_app
309+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextBatch(batch3))
310+
.unwrap();
311+
312+
// Verify we're still on page 0, but now have 160 rows total
313+
{
314+
let state = test_app.state();
315+
assert_eq!(state.flightsql_tab.current_page().unwrap(), 0);
316+
assert_eq!(state.flightsql_tab.total_loaded_rows(), 160);
317+
// Verify we still need more batches for page 1
318+
assert!(state.flightsql_tab.needs_more_batches_for_page(1));
319+
}
320+
321+
// Send fourth batch: another 40 rows (total 200, NOW enough for page 1!)
322+
// The system should automatically advance to page 1
323+
let batch4 = create_execution_results(&create_values_query_offset(40, 160)).await;
324+
test_app
325+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextBatch(batch4))
326+
.unwrap();
327+
328+
// Process the automatic NextPage event that was queued
329+
// In the real app, this happens automatically in the event loop
330+
// In tests, we need to simulate it by checking for and processing the event
331+
// Since we can't easily intercept the event queue in tests, we verify the state is ready
332+
{
333+
let state = test_app.state();
334+
assert_eq!(state.flightsql_tab.total_loaded_rows(), 200);
335+
// Verify we now have enough data for page 1
336+
assert!(!state.flightsql_tab.needs_more_batches_for_page(1));
337+
}
338+
339+
// Now when user manually advances (or automatic event processes), we can show page 1
340+
test_app
341+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextPage)
342+
.unwrap();
343+
344+
{
345+
let state = test_app.state();
346+
assert_eq!(state.flightsql_tab.current_page().unwrap(), 1);
347+
let page_results = state.flightsql_tab.current_page_results().unwrap();
348+
assert_eq!(page_results.num_rows(), 100); // Page 1 shows rows 100-199
349+
}
350+
}
351+
352+
// Tests that the system correctly handles the case where exactly enough batches
353+
// are loaded to fill a page (boundary condition)
354+
#[tokio::test]
355+
async fn exact_batches_for_page() {
356+
let mut test_app = TestApp::new().await;
357+
358+
test_app
359+
.handle_app_event(AppEvent::FlightSQLNewExecution)
360+
.unwrap();
361+
362+
// Send first batch: 50 rows
363+
let batch1 = create_execution_results(&create_values_query(50)).await;
364+
test_app
365+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextBatch(batch1))
366+
.unwrap();
367+
368+
// Verify page 0 shows 50 rows
369+
{
370+
let state = test_app.state();
371+
assert_eq!(state.flightsql_tab.current_page().unwrap(), 0);
372+
assert_eq!(state.flightsql_tab.total_loaded_rows(), 50);
373+
}
374+
375+
// Send second batch: another 50 rows (total 100, exactly fills page 0)
376+
let batch2 = create_execution_results(&create_values_query_offset(50, 50)).await;
377+
test_app
378+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextBatch(batch2))
379+
.unwrap();
380+
381+
// Verify page 0 now shows 100 rows
382+
{
383+
let state = test_app.state();
384+
assert_eq!(state.flightsql_tab.current_page().unwrap(), 0);
385+
let page_results = state.flightsql_tab.current_page_results().unwrap();
386+
assert_eq!(page_results.num_rows(), 100);
387+
assert_eq!(state.flightsql_tab.total_loaded_rows(), 100);
388+
}
389+
390+
// Send third batch: another 50 rows (total 150, NOT enough for full page 1)
391+
let batch3 = create_execution_results(&create_values_query_offset(50, 100)).await;
392+
test_app
393+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextBatch(batch3))
394+
.unwrap();
395+
396+
{
397+
let state = test_app.state();
398+
assert_eq!(state.flightsql_tab.total_loaded_rows(), 150);
399+
assert!(state.flightsql_tab.needs_more_batches_for_page(1));
400+
}
401+
402+
// Send fourth batch: exactly 50 more rows (total 200, exactly fills page 1)
403+
let batch4 = create_execution_results(&create_values_query_offset(50, 150)).await;
404+
test_app
405+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextBatch(batch4))
406+
.unwrap();
407+
408+
{
409+
let state = test_app.state();
410+
assert_eq!(state.flightsql_tab.total_loaded_rows(), 200);
411+
assert!(!state.flightsql_tab.needs_more_batches_for_page(1));
412+
}
413+
414+
// Advance to page 1
415+
test_app
416+
.handle_app_event(AppEvent::FlightSQLExecutionResultsNextPage)
417+
.unwrap();
418+
419+
{
420+
let state = test_app.state();
421+
assert_eq!(state.flightsql_tab.current_page().unwrap(), 1);
422+
let page_results = state.flightsql_tab.current_page_results().unwrap();
423+
assert_eq!(page_results.num_rows(), 100);
424+
}
425+
}

0 commit comments

Comments
 (0)