refactor(error): eliminate RwError usages in batch
#13557
Conversation
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
| #[error(transparent)] | ||
| Shared( | ||
| #[from] | ||
| #[backtrace] | ||
| Arc<Self>, | ||
| ), |
There was a problem hiding this comment.
What is this Shared for? Perhaps need some comments
Okay, it means Arc 😄
| #[backtrace] | ||
| BoxedError, | ||
| ), | ||
|
|
||
| #[error("Failed to send result to channel")] | ||
| SenderError, |
There was a problem hiding this comment.
The source error is dropped. Is this intended?
There was a problem hiding this comment.
I believe so. The error variant of channel sending result is for retrieving the ownership of the item back to the caller, which I think is not that meaningful here.
There was a problem hiding this comment.
Just checked UnsupportedFunction and found there is only 1 use case for it, furthurmore, I think that should be a panic instead of error.
risingwave/src/batch/src/executor/join/mod.rs
Lines 130 to 132 in b75c7e4
|
Thanks for the work! 🥰 |
Signed-off-by: Bugen Zhao <[email protected]>
| Self::Serde(m.into()) | ||
| } | ||
| } | ||
| impl From<ValueEncodingError> for BatchError { |
There was a problem hiding this comment.
| impl From<ValueEncodingError> for BatchError { | |
| impl From<ValueEncodingError> for BatchError { |
There was a problem hiding this comment.
I intended compact them since they're in the same group. 😰
src/batch/src/executor/source.rs
Outdated
| return Err(BatchError::Internal(anyhow!( | ||
| "Only support insert op in batch source executor" | ||
| )))); | ||
| ))); |
| use crate::task::channel::{ChanReceiver, ChanReceiverImpl, ChanSender, ChanSenderImpl}; | ||
| use crate::task::data_chunk_in_channel::DataChunkInChannel; | ||
|
|
||
| #[derive(Clone)] | ||
| pub struct ConsistentHashShuffleSender { | ||
| senders: Vec<mpsc::Sender<BatchSharedResult<Option<DataChunkInChannel>>>>, | ||
| senders: Vec<mpsc::Sender<SharedResult<Option<DataChunkInChannel>>>>, |
There was a problem hiding this comment.
Means Arc? 🤣 Prefer to just call it "Arc"
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #13557 +/- ##
==========================================
- Coverage 68.44% 68.37% -0.08%
==========================================
Files 1511 1511
Lines 259862 259852 -10
==========================================
- Hits 177856 177663 -193
- Misses 82006 82189 +183
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
#4077. Close #4368.
Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.