Skip to content

[Rust] Refactor RecordBatchStream trait#766

Merged
eddyxu merged 6 commits intomainfrom
lei/fragment_stream
Apr 11, 2023
Merged

[Rust] Refactor RecordBatchStream trait#766
eddyxu merged 6 commits intomainfrom
lei/fragment_stream

Conversation

@eddyxu
Copy link
Copy Markdown
Member

@eddyxu eddyxu commented Apr 11, 2023

Refactor to extract RecordBatchStream. There will be two implementation, one is DatasetRecordBatchStream (already implemented), and FragmentRecordBatchStream (to support #557).

@eddyxu eddyxu requested a review from changhiskhan April 11, 2023 02:09
@eddyxu eddyxu self-assigned this Apr 11, 2023
@eddyxu eddyxu requested a review from gsilvestrin April 11, 2023 02:21
Copy link
Copy Markdown
Contributor

@changhiskhan changhiskhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. just curious if the 'static trait bound is needed and if so why does the compiler want it

@eddyxu
Copy link
Copy Markdown
Member Author

eddyxu commented Apr 11, 2023

lgtm. just curious if the 'static trait bound is needed and if so why does the compiler want it

seems because stream is used in flat_search

Compiling lance v0.4.1 (/Users/lei/work/lance/rust)
error[E0310]: the parameter type `impl RecordBatchStream` may not live long enough
  --> src/io/exec/knn.rs:56:25
   |
56 |           let bg_thread = tokio::spawn(async move {
   |  _________________________^
57 | |             let batch = match flat_search(stream, &q).await {
58 | |                 Ok(b) => b,
59 | |                 Err(e) => {
...  |
74 | |             drop(tx);
75 | |         });
   | |__________^ ...so that the type `impl RecordBatchStream` will meet its required lifetime bounds
   |
help: consider adding an explicit lifetime bound...
   |
52 |     fn from_stream(stream: impl RecordBatchStream + 'static, query: &Query) -> Self {
   |                                                   +++++++++

@eddyxu eddyxu merged commit 01f7b2b into main Apr 11, 2023
@eddyxu eddyxu deleted the lei/fragment_stream branch April 11, 2023 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants