-
Notifications
You must be signed in to change notification settings - Fork 526
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
fix: iter only need the pk prefix reference instead of ownership #3126
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3126 +/- ##
==========================================
- Coverage 73.64% 73.64% -0.01%
==========================================
Files 736 736
Lines 101570 101581 +11
==========================================
+ Hits 74805 74809 +4
- Misses 26765 26772 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
let cell_based_bounds = (Unbounded, Unbounded); | ||
let mem_table_bounds: (Bound<Vec<u8>>, Bound<Vec<u8>>) = (Unbounded, Unbounded); | ||
let mem_table_iter = self.mem_table.buffer.range(mem_table_bounds); | ||
Ok(StateTableRowIter::into_stream( | ||
&self.keyspace, | ||
self.column_descs.clone(), | ||
mem_table_iter, | ||
cell_based_bounds, | ||
epoch, | ||
)) |
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.
Here I can not reuse self.iter(epoch)
. may be a bug of compiler
@@ -203,33 +203,46 @@ impl<S: StateStore> StateTable<S> { | |||
/// This function scans rows from the relational table with specific `pk_prefix`. | |||
pub async fn iter_with_pk_prefix( | |||
&self, | |||
pk_prefix: Row, | |||
pk_prefix: Option<&Row>, |
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.
When will the pk_prefix
be None
? Can we just reuse iter
? 😁
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.
Yes it's exactly self.iter(), I just copy the code. But seems like we can not simply write self.iter() due to some compiler problems (Checked this with @skyzh before).
BTW StateTableRowIter
seems needs some refactor, it may need to expose lifetime. Or we should just simply remove it.
What's changed and what's your intention?
As title
Checklist
./risedev check
(or alias,./risedev c
)Refer to a related PR or issue link (optional)