Skip to content
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!: migrate from thiserror to snafu #181

Merged
merged 5 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ rand_core = { version = "0.6", default-features = false }
rayon = { version = "1.5" }
serde = { version = "1", default-features = false }
serde_json = { version = "1" }
thiserror = { version = "1", default-features = false }
snafu = { version = "0.8.4", default-features = false, features = ["std"] }
tiny-keccak = { version = "2.0.2", features = [ "keccak" ] }
# tokio = { version = "1.39.3" }
tracing = { version = "0.1.36" }
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql-parser/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ bigdecimal = { workspace = true }
chrono = { workspace = true, features = ["serde"] }
lalrpop-util = { workspace = true, features = ["lexer", "unicode"] }
serde = { workspace = true, features = ["serde_derive", "alloc"] }
thiserror = { workspace = true }
snafu = { workspace = true }

[build-dependencies]
lalrpop = { workspace = true }
Expand Down
25 changes: 17 additions & 8 deletions crates/proof-of-sql-parser/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
use alloc::string::String;
use thiserror::Error;
use snafu::Snafu;

/// Errors encountered during the parsing process
#[derive(Debug, Error, Eq, PartialEq)]
#[derive(Debug, Snafu, Eq, PartialEq)]
pub enum ParseError {
#[error("Unable to parse query")]
#[snafu(display("Unable to parse query"))]
/// Cannot parse the query
QueryParseError(String),
#[error("Unable to parse identifier")]
QueryParseError {
/// The underlying error
error: String,
},
#[snafu(display("Unable to parse identifier"))]
/// Cannot parse the identifier
IdentifierParseError(String),
#[error("Unable to parse resource_id")]
IdentifierParseError {
/// The underlying error
error: String,
},
#[snafu(display("Unable to parse resource_id"))]
/// Can not parse the resource_id
ResourceIdParseError(String),
ResourceIdParseError {
/// The underlying error
error: String,
},
}

/// General parsing error that may occur, for example if the provided schema/object_name strings
Expand Down
4 changes: 2 additions & 2 deletions crates/proof-of-sql-parser/src/identifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ impl FromStr for Identifier {
fn from_str(string: &str) -> ParseResult<Self> {
let name = IdentifierParser::new()
.parse(string)
.map_err(|e| ParseError::IdentifierParseError(
format!("failed to parse identifier, (you may have used a reserved keyword as an ID, i.e. 'timestamp') {:?}", e)))?;
.map_err(|e| ParseError::IdentifierParseError{ error:
format!("failed to parse identifier, (you may have used a reserved keyword as an ID, i.e. 'timestamp') {:?}", e)})?;

Ok(Identifier::new(name))
}
Expand Down
12 changes: 6 additions & 6 deletions crates/proof-of-sql-parser/src/intermediate_ast_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1323,18 +1323,18 @@ fn we_cannot_parse_literals_outside_of_i128_range_in_the_result_expr() {
.is_ok());
assert_eq!(
"select 170141183460469231731687303715884105728 from tab".parse::<SelectStatement>(),
Err(super::error::ParseError::QueryParseError(
"i128 out of range".to_string()
))
Err(super::error::ParseError::QueryParseError {
error: "i128 out of range".to_string()
})
);
assert!("select -170141183460469231731687303715884105728 from tab"
.parse::<SelectStatement>()
.is_ok());
assert_eq!(
"select -170141183460469231731687303715884105729 from tab".parse::<SelectStatement>(),
Err(super::error::ParseError::QueryParseError(
"i128 out of range".to_string()
))
Err(super::error::ParseError::QueryParseError {
error: "i128 out of range".to_string()
})
);
}

Expand Down
21 changes: 12 additions & 9 deletions crates/proof-of-sql-parser/src/intermediate_decimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,30 @@ use alloc::string::String;
use bigdecimal::{num_bigint::BigInt, BigDecimal, ParseBigDecimalError, ToPrimitive};
use core::{fmt, hash::Hash, str::FromStr};
use serde::{Deserialize, Serialize};
use thiserror::Error;
use snafu::Snafu;

/// Errors related to the processing of decimal values in proof-of-sql
#[derive(Error, Debug, PartialEq)]
#[derive(Snafu, Debug, PartialEq)]
pub enum IntermediateDecimalError {
/// Represents an error encountered during the parsing of a decimal string.
#[error("{0}")]
ParseError(ParseBigDecimalError),
#[snafu(display("{error}"))]
ParseError {
/// The underlying error
error: ParseBigDecimalError,
},
/// Error occurs when this decimal cannot fit in a primitive.
#[error("Value out of range for target type")]
#[snafu(display("Value out of range for target type"))]
OutOfRange,
/// Error occurs when this decimal cannot be losslessly cast into a primitive.
#[error("Fractional part of decimal is non-zero")]
#[snafu(display("Fractional part of decimal is non-zero"))]
LossyCast,
/// Cannot cast this decimal to a big integer
#[error("Conversion to integer failed")]
#[snafu(display("Conversion to integer failed"))]
ConversionFailure,
}
impl From<ParseBigDecimalError> for IntermediateDecimalError {
fn from(value: ParseBigDecimalError) -> Self {
IntermediateDecimalError::ParseError(value)
IntermediateDecimalError::ParseError { error: value }
}
}

Expand Down Expand Up @@ -88,7 +91,7 @@ impl FromStr for IntermediateDecimal {
.map(|value| IntermediateDecimal {
value: value.normalized(),
})
.map_err(ParseError)
.map_err(|err| ParseError { error: err })
}
}

Expand Down
43 changes: 29 additions & 14 deletions crates/proof-of-sql-parser/src/posql_time/error.rs
Original file line number Diff line number Diff line change
@@ -1,41 +1,56 @@
use alloc::string::{String, ToString};
use serde::{Deserialize, Serialize};
use thiserror::Error;
use snafu::Snafu;

/// Errors related to time operations, including timezone and timestamp conversions.s
#[derive(Error, Debug, Eq, PartialEq, Serialize, Deserialize)]
#[derive(Snafu, Debug, Eq, PartialEq, Serialize, Deserialize)]
pub enum PoSQLTimestampError {
/// Error when the timezone string provided cannot be parsed into a valid timezone.
#[error("invalid timezone string: {0}")]
InvalidTimezone(String),
#[snafu(display("invalid timezone string: {timezone}"))]
InvalidTimezone {
/// The invalid timezone
timezone: String,
},

/// Error indicating an invalid timezone offset was provided.
#[error("invalid timezone offset")]
#[snafu(display("invalid timezone offset"))]
InvalidTimezoneOffset,

/// Indicates a failure to convert between different representations of time units.
#[error("Invalid time unit")]
InvalidTimeUnit(String),
#[snafu(display("Invalid time unit"))]
InvalidTimeUnit {
/// The underlying error
error: String,
},

/// The local time does not exist because there is a gap in the local time.
/// This variant may also be returned if there was an error while resolving the local time,
/// caused by for example missing time zone data files, an error in an OS API, or overflow.
#[error("Local time does not exist because there is a gap in the local time")]
#[snafu(display("Local time does not exist because there is a gap in the local time"))]
LocalTimeDoesNotExist,

/// The local time is ambiguous because there is a fold in the local time.
/// This variant contains the two possible results, in the order (earliest, latest).
#[error("Unix timestamp is ambiguous because there is a fold in the local time.")]
Ambiguous(String),
#[snafu(display("Unix timestamp is ambiguous because there is a fold in the local time."))]
Ambiguous {
/// The underlying error
error: String,
},

/// Represents a catch-all for parsing errors not specifically covered by other variants.
#[error("Timestamp parsing error: {0}")]
ParsingError(String),
#[snafu(display("Timestamp parsing error: {error}"))]
ParsingError {
/// The underlying error
error: String,
},

/// Represents a failure to parse a provided time unit precision value, PoSQL supports
/// Seconds, Milliseconds, Microseconds, and Nanoseconds
#[error("Timestamp parsing error: {0}")]
UnsupportedPrecision(String),
#[snafu(display("Timestamp parsing error: {error}"))]
UnsupportedPrecision {
/// The underlying error
error: String,
},
}

// This exists because TryFrom<DataType> for ColumnType error is String
Expand Down
22 changes: 14 additions & 8 deletions crates/proof-of-sql-parser/src/posql_time/timestamp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,11 @@ impl PoSQLTimestamp {
/// assert_eq!(intermediate_timestamp.timezone(), PoSQLTimeZone::FixedOffset(10800)); // 3 hours in seconds
/// ```
pub fn try_from(timestamp_str: &str) -> Result<Self, PoSQLTimestampError> {
let dt = DateTime::parse_from_rfc3339(timestamp_str)
.map_err(|e| PoSQLTimestampError::ParsingError(e.to_string()))?;
let dt = DateTime::parse_from_rfc3339(timestamp_str).map_err(|e| {
PoSQLTimestampError::ParsingError {
error: e.to_string(),
}
})?;

let offset_seconds = dt.offset().local_minus_utc();
let timezone = PoSQLTimeZone::from_offset(offset_seconds);
Expand Down Expand Up @@ -108,9 +111,9 @@ impl PoSQLTimestamp {
timeunit: PoSQLTimeUnit::Second,
timezone: PoSQLTimeZone::Utc,
}),
LocalResult::Ambiguous(earliest, latest) => Err(PoSQLTimestampError::Ambiguous(
LocalResult::Ambiguous(earliest, latest) => Err(PoSQLTimestampError::Ambiguous{ error:
format!("The local time is ambiguous because there is a fold in the local time: earliest: {} latest: {} ", earliest, latest),
)),
}),
LocalResult::None => Err(PoSQLTimestampError::LocalTimeDoesNotExist),
}
}
Expand Down Expand Up @@ -177,9 +180,9 @@ mod tests {
let input = "not-a-timestamp";
assert_eq!(
PoSQLTimestamp::try_from(input),
Err(PoSQLTimestampError::ParsingError(
"input contains invalid characters".into()
))
Err(PoSQLTimestampError::ParsingError {
error: "input contains invalid characters".into()
})
);
}

Expand All @@ -198,7 +201,10 @@ mod tests {
// This test assumes that there's a catch-all parsing error case that isn't covered by the more specific errors.
let malformed_input = "2009-01-03T::00Z"; // Intentionally malformed timestamp
let result = PoSQLTimestamp::try_from(malformed_input);
assert!(matches!(result, Err(PoSQLTimestampError::ParsingError(_))));
assert!(matches!(
result,
Err(PoSQLTimestampError::ParsingError { .. })
));
}

#[test]
Expand Down
4 changes: 3 additions & 1 deletion crates/proof-of-sql-parser/src/posql_time/timezone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ impl TryFrom<&Option<Arc<str>>> for PoSQLTimeZone {
let total_seconds = sign * ((hours * 3600) + (minutes * 60));
Ok(PoSQLTimeZone::FixedOffset(total_seconds))
}
_ => Err(PoSQLTimestampError::InvalidTimezone(tz.to_string())),
_ => Err(PoSQLTimestampError::InvalidTimezone {
timezone: tz.to_string(),
}),
}
}
None => Ok(PoSQLTimeZone::Utc),
Expand Down
6 changes: 4 additions & 2 deletions crates/proof-of-sql-parser/src/posql_time/unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ impl TryFrom<&str> for PoSQLTimeUnit {
"3" => Ok(PoSQLTimeUnit::Millisecond),
"6" => Ok(PoSQLTimeUnit::Microsecond),
"9" => Ok(PoSQLTimeUnit::Nanosecond),
_ => Err(PoSQLTimestampError::UnsupportedPrecision(value.into())),
_ => Err(PoSQLTimestampError::UnsupportedPrecision {
error: value.into(),
}),
}
}
}
Expand Down Expand Up @@ -65,7 +67,7 @@ mod time_unit_tests {
let result = PoSQLTimeUnit::try_from(value);
assert!(matches!(
result,
Err(PoSQLTimestampError::UnsupportedPrecision(_))
Err(PoSQLTimestampError::UnsupportedPrecision { .. })
));
}
}
Expand Down
8 changes: 5 additions & 3 deletions crates/proof-of-sql-parser/src/resource_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,11 @@ impl FromStr for ResourceId {
type Err = ParseError;

fn from_str(string: &str) -> ParseResult<Self> {
let (schema, object_name) = ResourceIdParser::new()
.parse(string)
.map_err(|e| ParseError::ResourceIdParseError(format!("{:?}", e)))?;
let (schema, object_name) = ResourceIdParser::new().parse(string).map_err(|e| {
ParseError::ResourceIdParseError {
error: format!("{:?}", e),
}
})?;

// use unsafe `Identifier::new` to prevent double parsing the ids
Ok(ResourceId {
Expand Down
4 changes: 3 additions & 1 deletion crates/proof-of-sql-parser/src/select_statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ impl FromStr for SelectStatement {
fn from_str(query: &str) -> ParseResult<Self> {
SelectStatementParser::new()
.parse(query)
.map_err(|e| ParseError::QueryParseError(e.to_string()))
.map_err(|e| ParseError::QueryParseError {
error: e.to_string(),
})
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ rand = { workspace = true, default-features = false, optional = true }
rayon = { workspace = true, optional = true }
serde = { workspace = true, features = ["serde_derive"] }
serde_json = { workspace = true }
thiserror = { workspace = true }
snafu = { workspace = true }
tiny-keccak = { workspace = true }
tracing = { workspace = true, features = ["attributes"] }
zerocopy = { workspace = true }
Expand Down
29 changes: 19 additions & 10 deletions crates/proof-of-sql/src/base/commitment/column_bounds.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use super::committable_column::CommittableColumn;
use alloc::boxed::Box;
use serde::{Deserialize, Serialize};
use thiserror::Error;
use snafu::Snafu;

/// Cannot construct bounds where min is greater than max.
#[derive(Error, Debug)]
#[error("cannot construct bounds where min is greater than max")]
#[derive(Snafu, Debug)]
#[snafu(display("cannot construct bounds where min is greater than max"))]
pub struct NegativeBounds;

/// Inner value for [`Bounds::Sharp`] and [`Bounds::Bounded`].
Expand Down Expand Up @@ -187,9 +187,14 @@ where
}

/// Columns with different [`ColumnBounds`] variants cannot operate with each other.
#[derive(Debug, Error)]
#[error("column with bounds {0:?} cannot operate with column with bounds {1:?}")]
pub struct ColumnBoundsMismatch(Box<ColumnBounds>, Box<ColumnBounds>);
#[derive(Debug, Snafu)]
#[snafu(display(
"column with bounds {bounds_a:?} cannot operate with column with bounds {bounds_b:?}"
))]
pub struct ColumnBoundsMismatch {
bounds_a: Box<ColumnBounds>,
bounds_b: Box<ColumnBounds>,
}

/// Column metadata storing the bounds for column types that have order.
///
Expand Down Expand Up @@ -254,9 +259,10 @@ impl ColumnBounds {
(ColumnBounds::Int128(bounds_a), ColumnBounds::Int128(bounds_b)) => {
Ok(ColumnBounds::Int128(bounds_a.union(bounds_b)))
}
(bounds_a, bounds_b) => {
Err(ColumnBoundsMismatch(Box::new(bounds_a), Box::new(bounds_b)))
}
(bounds_a, bounds_b) => Err(ColumnBoundsMismatch {
bounds_a: Box::new(bounds_a),
bounds_b: Box::new(bounds_b),
}),
}
}

Expand All @@ -282,7 +288,10 @@ impl ColumnBounds {
(ColumnBounds::TimestampTZ(bounds_a), ColumnBounds::TimestampTZ(bounds_b)) => {
Ok(ColumnBounds::TimestampTZ(bounds_a.difference(bounds_b)))
}
(_, _) => Err(ColumnBoundsMismatch(Box::new(self), Box::new(other))),
(_, _) => Err(ColumnBoundsMismatch {
bounds_a: Box::new(self),
bounds_b: Box::new(other),
}),
}
}
}
Expand Down
Loading