Conversation
eitsupi
left a comment
There was a problem hiding this comment.
I have a few high-level questions/concerns about the current approach.
SQL support
Is there a reason why the SQL abstraction cannot be left to dbplyr?
Maintaining SQL Server-specific query generation in vscode-R looks potentially fragile to me.
arrow dplyr query
I am also a little concerned about the amount of work being done when filtering or sorting.
For a complex lazy dplyr query, it looks like some expensive parts of the query could be evaluated repeatedly as the viewer state changes.
I may be particularly sensitive to this because I have contributed to the Arrow R package a number of times.
Data conversion
Arrow has a richer type system than R data frames, so it feels unfortunate to convert Arrow data back into R data frames as part of the viewing pipeline.
Would it make sense to preserve Arrow data through to the JS side instead?
Embedding DuckDB would be one relatively easy way to process Arrow data there, although of course DuckDB does not support every Arrow type either.
Thanks for the review @eitsupi. Those concerns are valid. The main design choices are around the data viewer layer for forward/backward scrolling, including those operations after filtering and sorting and keeping them responsive and efficient. For SQL ServerYou are right; some SQL Server-specific abstractions could essentially be handed down to One special case is arrow_dplyr_queryThe major work on the arrow side is actually around The more expensive re-execution can happen when the viewer filter or sort changes, and the requested page is then fetched separately. So for a complex I thought about disabling filtering and sorting for Any improvement ideas are welcome, especially around whether the evaluated state can be reused without losing the random access and backward scrolling. Maybe we can also have a list of operations that can be supported and disabled. As for |
|
I think I may just disable filtering and sorting for lazy queries such as |
Closes #1785
This might be a big change to support both
Arrow'sFileSystemDatasetandSQL Serverin the Data Viewer. I submit it here for review and discussion. It is not necessarily intended to adopt both of them. The implementations are more complicated than I initially thought, especially when performance is one part of the goals. Nevertheless, the following decisions are mine, with GPT used to generate and iterate on the implementation here.Summary
Add lazy/on-demand Data Viewer support for large Arrow and DBI-backed data sources without loading the full dataset into memory.
Arrow and DBI now share the same general paging model:
dplyr::arrange()).The existing in-memory Data Viewer behaviour remains unchanged.
Changes
Arrow
integer64, dates, datetimes, durations, and nested columns.DBI
tbl_sqlobjects, currently targeting SQL Server.dbFetch()for continuous forward scrolling.OFFSETto reposition the result for jumps or backward requests outside the cache.ORDER BYclauses inside SQL Server subqueries where they are invalid.Other changes
sess/R/handlers.Rsess/R/server.Rsesscontinues processing subsequent requests after an interrupted Data Viewer fetch.src/session.tssess/DESCRIPTIONData Viewer tests
Arrow and DBI use backend-specific strategies under the same Data Viewer behaviour: Arrow keeps source-row mappings for efficient random access, while DBI lets SQL Server perform filtering and sorting and streams the resulting rows through an open database result.
Validation
Added regression tests covering: