datafusion-tui icon indicating copy to clipboard operation
datafusion-tui copied to clipboard

Add pagination to query Execution

Open matthewmturner opened this issue 1 year ago • 7 comments

This PR adds pagination to query execution.

  • [x] Adds a new PaginatingRecordBatchStream
  • [ ] Adds new UI and key handlers for going to the next and previous pagaes

matthewmturner avatar Sep 13 '24 13:09 matthewmturner

@alamb not done yet, but thought you may be interested.

matthewmturner avatar Sep 13 '24 14:09 matthewmturner

Good news - i got query results to display that are plugged into the new paginated stream. However, this is a quite invasive change. Theres still a lot to cleanup and im not sure yet whether this is the desired structure. That being said, im quite happy to have an end to end flow working.

image

matthewmturner avatar Sep 14 '24 14:09 matthewmturner

So I ended up going with a different approach on paginating but i think it aligns better with the rendering model.

Now, the sql tab state will store the record batches and current index to display and those will be updated synchronously in AppEvent handlers. And AppExecution will just have Arc<Mutex<Option<SendableRecordBatchStream>>>. Pagination on this will be done in tokio tasks and when batch is available will be sent in AppEvent::QueryResultNextPage(batch). This means that rendering will no longer require a lock to get access to the current batch as was required in the previous model. I still need to update several places to work with with this model but I did successfully test that it worked correctly for a single query to display results and im pretty sure it will work for the remaining places.

matthewmturner avatar Sep 15 '24 14:09 matthewmturner

. This means that rendering will no longer require a lock to get access to the current batch as was required in the previous model. I

This makes sense to me

alamb avatar Sep 15 '24 15:09 alamb

Making good progress

https://github.com/user-attachments/assets/2ad4a5ea-2dce-49c4-bce7-cede0ae69eb7

Edit: Sorry its kind of blurry but im pressing the right arrow and youll see the results updating and page number changing.

Still more to clean up and improve but its all coming together now

matthewmturner avatar Sep 17 '24 13:09 matthewmturner

Might end up needing those arrow utilities sooner than expected because the FlightSQL client doesnt let us set the record batch size.

matthewmturner avatar Sep 19 '24 12:09 matthewmturner

BTW I am hoping to next contribute:

  1. An upgrade to datafusion 42 (https://github.com/datafusion-contrib/datafusion-functions-json is updated but I haven't checked the others)
  2. Then local file support
  3. Some sort of test for S3 integration (likely based on localstack) / Delta integration

alamb avatar Sep 19 '24 12:09 alamb