-
Notifications
You must be signed in to change notification settings - Fork 328
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: add scan_to_stream()
to Table trait to postpone the stream generation
#1639
Conversation
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a temporary regression due to the lack of an optimization rule.
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Codecov Report
@@ Coverage Diff @@
## develop #1639 +/- ##
===========================================
- Coverage 85.73% 85.48% -0.26%
===========================================
Files 566 566
Lines 90433 90795 +362
===========================================
+ Hits 77537 77614 +77
- Misses 12896 13181 +285 |
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Looks like the failing CI checks checked out an unrelated commit id |
…m generation (GreptimeTeam#1639) * add scan_to_stream to Table Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * impl parquet stream Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * reorganise adapters Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * implement scan_to_stream for mito table Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * clean up Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * add location info Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * fix: table scan * UT pass Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * impl project record batch Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * fix information schema Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * fix clippy Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * resolve CR comments Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * remove one todo Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * fix errors generated by merge commit Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * add output_ordering method to record batch stream Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * fix rustfmt Signed-off-by: Ruihang Xia <waynestxia@gmail.com> * enhance error types Signed-off-by: Ruihang Xia <waynestxia@gmail.com> --------- Signed-off-by: Ruihang Xia <waynestxia@gmail.com> Co-authored-by: Lei, HUANG <mrsatangel@gmail.com>
I hereby agree to the terms of the GreptimeDB CLA
What's changed and what's your intention?
Intension
scan()
method onTable
returns a physical plan (execution plan in DF) directly. This manner has several limitations:Table
implementations are usually simply adapted to DataFusion's table source, and it is hard to add new optionsscan()
.Changes
Add a new interface
scan_to_stream()
to replace the existingscan()
. It returns a stream that can be adapted to execution plan. And leverage the middle layerDfTableProviderAdapter
as the only entry point of all theTable
implementations. It holds aTable
reference and a mutableScanRequest
that can be changed during the optimization phase. We can further postpone the call toscan_to_stream()
to the return value ofDfTableProviderAdapter::scan()
, to avoid I/O during planning.The main change is the new interface
scan_to_stream()
and deprecatingscan()
.InformationSchema
and file table engine have some noticeable changes due to this.Checklist
Refer to a related PR or issue link (optional)