-
Notifications
You must be signed in to change notification settings - Fork 585
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(expr): introduce ExprError #3081
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
// Copyright 2022 Singularity Data | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
pub use anyhow::anyhow; | ||
use risingwave_common::error::{ErrorCode, RwError}; | ||
use risingwave_common::types::DataType; | ||
use thiserror::Error; | ||
|
||
#[derive(Error, Debug)] | ||
pub enum ExprError { | ||
#[error("Unsupported function: {0}")] | ||
UnsupportedFunction(String), | ||
|
||
#[error("Can't cast {0} to {1}")] | ||
Cast(&'static str, &'static str), | ||
|
||
// TODO: Unify Cast and Cast2. | ||
#[error("Can't cast {0:?} to {1:?}")] | ||
Cast2(DataType, DataType), | ||
|
||
#[error("Out of range")] | ||
NumericOutOfRange, | ||
|
||
#[error("Parse error: {0}")] | ||
Parse(&'static str), | ||
|
||
#[error("Invalid parameter {name}: {reason}")] | ||
InvalidParam { name: &'static str, reason: String }, | ||
|
||
#[error("Array error: {0}")] | ||
Array( | ||
#[backtrace] | ||
#[source] | ||
RwError, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will we refactor this later? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes
Comment on lines
+43
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May review this in the future. 🤣 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need ArrayError :( |
||
), | ||
|
||
#[error(transparent)] | ||
Internal(#[from] anyhow::Error), | ||
} | ||
|
||
#[macro_export] | ||
macro_rules! ensure { | ||
($cond:expr $(,)?) => { | ||
if !$cond { | ||
return Err($crate::ExprError::Internal($crate::error::anyhow!( | ||
stringify!($cond) | ||
))); | ||
} | ||
}; | ||
($cond:expr, $msg:literal $(,)?) => { | ||
if !$cond { | ||
return Err($crate::ExprError::Internal($crate::error::anyhow!($msg))); | ||
} | ||
}; | ||
($cond:expr, $err:expr $(,)?) => { | ||
if !$cond { | ||
return Err($crate::ExprError::Internal($crate::error::anyhow!$err)); | ||
} | ||
}; | ||
($cond:expr, $fmt:expr, $($arg:tt)*) => { | ||
if !$cond { | ||
return Err($crate::ExprError::Internal($crate::error::anyhow!($fmt, $($arg)*))); | ||
} | ||
}; | ||
} | ||
|
||
impl From<ExprError> for RwError { | ||
fn from(s: ExprError) -> Self { | ||
ErrorCode::ExprError(Box::new(s)).into() | ||
} | ||
} | ||
|
||
#[macro_export] | ||
macro_rules! bail { | ||
($msg:literal $(,)?) => { | ||
return Err($crate::ExprError::Internal($crate::error::anyhow!($msg))) | ||
}; | ||
($err:expr $(,)?) => { | ||
return Err($crate::ExprError::Internal($crate::error::anyhow!($err))) | ||
}; | ||
($fmt:expr, $($arg:tt)*) => { | ||
return Err($crate::ExprError::Internal($crate::error::anyhow!($fmt, $($arg)*))) | ||
}; | ||
} |
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.
I'm not sure should we introduce
ExprError(ExprError)
here instead of convert to other RwError.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.
So there the error is boxed into a trait object. 🤔
ExprError(ExprError)
looks better to me.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.
I think the expression error will always be capsulated by some other error like
StreamError
when exposed to the user, so there might be no need to make it a variant ofRwError
eventually.We may keep it for now due to compatibility.
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.
It seems we can't refer
ExprError
here due to crate dependencies?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, same as
StorageError