diff --git a/Cargo.toml b/Cargo.toml index 683f5d4ec..9945ce97a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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 } tiny-keccak = { version = "2.0.2", features = [ "keccak" ] } # tokio = { version = "1.39.3" } tracing = { version = "0.1.36" } diff --git a/crates/proof-of-sql-parser/Cargo.toml b/crates/proof-of-sql-parser/Cargo.toml index 7254743df..228833b7e 100644 --- a/crates/proof-of-sql-parser/Cargo.toml +++ b/crates/proof-of-sql-parser/Cargo.toml @@ -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 } diff --git a/crates/proof-of-sql-parser/src/error.rs b/crates/proof-of-sql-parser/src/error.rs index e1449cc7d..e412df51a 100644 --- a/crates/proof-of-sql-parser/src/error.rs +++ b/crates/proof-of-sql-parser/src/error.rs @@ -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 diff --git a/crates/proof-of-sql-parser/src/identifier.rs b/crates/proof-of-sql-parser/src/identifier.rs index 949df4709..a5069aa28 100644 --- a/crates/proof-of-sql-parser/src/identifier.rs +++ b/crates/proof-of-sql-parser/src/identifier.rs @@ -45,8 +45,8 @@ impl FromStr for Identifier { fn from_str(string: &str) -> ParseResult { 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)) } diff --git a/crates/proof-of-sql-parser/src/intermediate_ast_tests.rs b/crates/proof-of-sql-parser/src/intermediate_ast_tests.rs index 0afae9a67..61dd38aa4 100644 --- a/crates/proof-of-sql-parser/src/intermediate_ast_tests.rs +++ b/crates/proof-of-sql-parser/src/intermediate_ast_tests.rs @@ -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::(), - 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::() .is_ok()); assert_eq!( "select -170141183460469231731687303715884105729 from tab".parse::(), - Err(super::error::ParseError::QueryParseError( - "i128 out of range".to_string() - )) + Err(super::error::ParseError::QueryParseError { + error: "i128 out of range".to_string() + }) ); } diff --git a/crates/proof-of-sql-parser/src/intermediate_decimal.rs b/crates/proof-of-sql-parser/src/intermediate_decimal.rs index d88e9041d..9ebaeb04e 100644 --- a/crates/proof-of-sql-parser/src/intermediate_decimal.rs +++ b/crates/proof-of-sql-parser/src/intermediate_decimal.rs @@ -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 for IntermediateDecimalError { fn from(value: ParseBigDecimalError) -> Self { - IntermediateDecimalError::ParseError(value) + IntermediateDecimalError::ParseError { error: value } } } @@ -88,7 +91,7 @@ impl FromStr for IntermediateDecimal { .map(|value| IntermediateDecimal { value: value.normalized(), }) - .map_err(ParseError) + .map_err(|err| ParseError { error: err }) } } diff --git a/crates/proof-of-sql-parser/src/posql_time/error.rs b/crates/proof-of-sql-parser/src/posql_time/error.rs index 1e07023d7..9e23cd019 100644 --- a/crates/proof-of-sql-parser/src/posql_time/error.rs +++ b/crates/proof-of-sql-parser/src/posql_time/error.rs @@ -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 for ColumnType error is String diff --git a/crates/proof-of-sql-parser/src/posql_time/timestamp.rs b/crates/proof-of-sql-parser/src/posql_time/timestamp.rs index 6807388c5..87e7b59da 100644 --- a/crates/proof-of-sql-parser/src/posql_time/timestamp.rs +++ b/crates/proof-of-sql-parser/src/posql_time/timestamp.rs @@ -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 { - 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); @@ -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), } } @@ -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() + }) ); } @@ -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] diff --git a/crates/proof-of-sql-parser/src/posql_time/timezone.rs b/crates/proof-of-sql-parser/src/posql_time/timezone.rs index 459195347..b7b9cee20 100644 --- a/crates/proof-of-sql-parser/src/posql_time/timezone.rs +++ b/crates/proof-of-sql-parser/src/posql_time/timezone.rs @@ -45,7 +45,9 @@ impl TryFrom<&Option>> 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), diff --git a/crates/proof-of-sql-parser/src/posql_time/unit.rs b/crates/proof-of-sql-parser/src/posql_time/unit.rs index 28b7739e1..97b1d87e1 100644 --- a/crates/proof-of-sql-parser/src/posql_time/unit.rs +++ b/crates/proof-of-sql-parser/src/posql_time/unit.rs @@ -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(), + }), } } } @@ -65,7 +67,7 @@ mod time_unit_tests { let result = PoSQLTimeUnit::try_from(value); assert!(matches!( result, - Err(PoSQLTimestampError::UnsupportedPrecision(_)) + Err(PoSQLTimestampError::UnsupportedPrecision { .. }) )); } } diff --git a/crates/proof-of-sql-parser/src/resource_id.rs b/crates/proof-of-sql-parser/src/resource_id.rs index 4f42c8474..3c4348cc2 100644 --- a/crates/proof-of-sql-parser/src/resource_id.rs +++ b/crates/proof-of-sql-parser/src/resource_id.rs @@ -91,9 +91,11 @@ impl FromStr for ResourceId { type Err = ParseError; fn from_str(string: &str) -> ParseResult { - 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 { diff --git a/crates/proof-of-sql-parser/src/select_statement.rs b/crates/proof-of-sql-parser/src/select_statement.rs index b64a40825..283e0820f 100644 --- a/crates/proof-of-sql-parser/src/select_statement.rs +++ b/crates/proof-of-sql-parser/src/select_statement.rs @@ -62,7 +62,9 @@ impl FromStr for SelectStatement { fn from_str(query: &str) -> ParseResult { SelectStatementParser::new() .parse(query) - .map_err(|e| ParseError::QueryParseError(e.to_string())) + .map_err(|e| ParseError::QueryParseError { + error: e.to_string(), + }) } } diff --git a/crates/proof-of-sql/Cargo.toml b/crates/proof-of-sql/Cargo.toml index 496f14a8d..a927077f6 100644 --- a/crates/proof-of-sql/Cargo.toml +++ b/crates/proof-of-sql/Cargo.toml @@ -44,7 +44,7 @@ rand = { workspace = true, default-features = false, optional = true } rayon = { workspace = 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 } @@ -75,6 +75,7 @@ development = ["arrow-csv"] default = ["arrow", "blitzar"] arrow = ["dep:arrow"] test = ["dep:rand"] +std = ["snafu/std"] [lints] workspace = true @@ -85,7 +86,7 @@ required-features = [ "blitzar", "test" ] [[example]] name = "posql_db" -required-features = [ "arrow", "blitzar" ] +required-features = [ "arrow", "blitzar", "std" ] [[bench]] name = "posql_benches" diff --git a/crates/proof-of-sql/src/base/commitment/column_bounds.rs b/crates/proof-of-sql/src/base/commitment/column_bounds.rs index d71d0a0fd..10272afd6 100644 --- a/crates/proof-of-sql/src/base/commitment/column_bounds.rs +++ b/crates/proof-of-sql/src/base/commitment/column_bounds.rs @@ -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`]. @@ -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, Box); +#[derive(Debug, Snafu)] +#[snafu(display( + "column with bounds {bounds_a:?} cannot operate with column with bounds {bounds_b:?}" +))] +pub struct ColumnBoundsMismatch { + bounds_a: Box, + bounds_b: Box, +} /// Column metadata storing the bounds for column types that have order. /// @@ -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), + }), } } @@ -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), + }), } } } diff --git a/crates/proof-of-sql/src/base/commitment/column_commitment_metadata.rs b/crates/proof-of-sql/src/base/commitment/column_commitment_metadata.rs index 4088f5e39..394c4cb78 100644 --- a/crates/proof-of-sql/src/base/commitment/column_commitment_metadata.rs +++ b/crates/proof-of-sql/src/base/commitment/column_commitment_metadata.rs @@ -2,20 +2,28 @@ use super::{column_bounds::BoundsInner, committable_column::CommittableColumn, C use crate::base::database::ColumnType; use core::fmt::Debug; use serde::{Deserialize, Serialize}; -use thiserror::Error; +use snafu::Snafu; /// Errors that can occur when constructing invalid [`ColumnCommitmentMetadata`]. -#[derive(Debug, Error)] +#[derive(Debug, Snafu)] pub enum InvalidColumnCommitmentMetadata { /// Column of this type cannot have these bounds. - #[error("column of type {0} cannot have bounds like {1:?}")] - TypeBoundsMismatch(ColumnType, ColumnBounds), + #[snafu(display("column of type {column_type} cannot have bounds like {column_bounds:?}"))] + TypeBoundsMismatch { + column_type: ColumnType, + column_bounds: ColumnBounds, + }, } /// During column operation, metadata indicates that the operand columns cannot be the same. -#[derive(Debug, Error)] -#[error("column with type {0} cannot operate with column with type {1}")] -pub struct ColumnCommitmentMetadataMismatch(ColumnType, ColumnType); +#[derive(Debug, Snafu)] +#[snafu(display( + "column with type {datatype_a} cannot operate with column with type {datatype_b}" +))] +pub struct ColumnCommitmentMetadataMismatch { + datatype_a: ColumnType, + datatype_b: ColumnType, +} const EXPECT_BOUNDS_MATCH_MESSAGE: &str = "we've already checked the column types match, which is a stronger requirement (mapping of type variants to bounds variants is surjective)"; @@ -51,10 +59,10 @@ impl ColumnCommitmentMetadata { column_type, bounds, }), - _ => Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch( + _ => Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch { column_type, - bounds, - )), + column_bounds: bounds, + }), } } @@ -117,10 +125,10 @@ impl ColumnCommitmentMetadata { other: ColumnCommitmentMetadata, ) -> Result { if self.column_type != other.column_type { - return Err(ColumnCommitmentMetadataMismatch( - self.column_type, - other.column_type, - )); + return Err(ColumnCommitmentMetadataMismatch { + datatype_a: self.column_type, + datatype_b: other.column_type, + }); } let bounds = self @@ -143,10 +151,10 @@ impl ColumnCommitmentMetadata { other: ColumnCommitmentMetadata, ) -> Result { if self.column_type != other.column_type { - return Err(ColumnCommitmentMetadataMismatch( - self.column_type, - other.column_type, - )); + return Err(ColumnCommitmentMetadataMismatch { + datatype_a: self.column_type, + datatype_b: other.column_type, + }); } let bounds = self @@ -267,14 +275,14 @@ mod tests { ColumnType::Boolean, ColumnBounds::BigInt(Bounds::Empty) ), - Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch(..)) + Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch { .. }) )); assert!(matches!( ColumnCommitmentMetadata::try_new( ColumnType::Boolean, ColumnBounds::Int128(Bounds::Empty) ), - Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch(..)) + Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch { .. }) )); assert!(matches!( @@ -282,14 +290,14 @@ mod tests { ColumnType::Decimal75(Precision::new(10).unwrap(), 10), ColumnBounds::Int128(Bounds::Empty) ), - Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch(..)) + Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch { .. }) )); assert!(matches!( ColumnCommitmentMetadata::try_new( ColumnType::Decimal75(Precision::new(10).unwrap(), 10), ColumnBounds::BigInt(Bounds::Empty) ), - Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch(..)) + Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch { .. }) )); assert!(matches!( @@ -297,14 +305,14 @@ mod tests { ColumnType::Scalar, ColumnBounds::BigInt(Bounds::Empty) ), - Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch(..)) + Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch { .. }) )); assert!(matches!( ColumnCommitmentMetadata::try_new( ColumnType::Scalar, ColumnBounds::Int128(Bounds::Empty) ), - Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch(..)) + Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch { .. }) )); assert!(matches!( @@ -312,11 +320,11 @@ mod tests { ColumnType::BigInt, ColumnBounds::Int128(Bounds::Empty) ), - Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch(..)) + Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch { .. }) )); assert!(matches!( ColumnCommitmentMetadata::try_new(ColumnType::BigInt, ColumnBounds::NoOrder), - Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch(..)) + Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch { .. }) )); assert!(matches!( @@ -324,11 +332,11 @@ mod tests { ColumnType::Int128, ColumnBounds::BigInt(Bounds::Empty) ), - Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch(..)) + Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch { .. }) )); assert!(matches!( ColumnCommitmentMetadata::try_new(ColumnType::Int128, ColumnBounds::NoOrder), - Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch(..)) + Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch { .. }) )); assert!(matches!( @@ -336,14 +344,14 @@ mod tests { ColumnType::VarChar, ColumnBounds::BigInt(Bounds::Empty) ), - Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch(..)) + Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch { .. }) )); assert!(matches!( ColumnCommitmentMetadata::try_new( ColumnType::VarChar, ColumnBounds::Int128(Bounds::Empty) ), - Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch(..)) + Err(InvalidColumnCommitmentMetadata::TypeBoundsMismatch { .. }) )); } diff --git a/crates/proof-of-sql/src/base/commitment/column_commitment_metadata_map.rs b/crates/proof-of-sql/src/base/commitment/column_commitment_metadata_map.rs index dda1f5445..f40e5d247 100644 --- a/crates/proof-of-sql/src/base/commitment/column_commitment_metadata_map.rs +++ b/crates/proof-of-sql/src/base/commitment/column_commitment_metadata_map.rs @@ -6,25 +6,35 @@ use crate::base::database::ColumnField; use alloc::string::{String, ToString}; use indexmap::IndexMap; use proof_of_sql_parser::Identifier; -use thiserror::Error; +use snafu::Snafu; /// Mapping of column identifiers to column metadata used to associate metadata with commitments. pub type ColumnCommitmentMetadataMap = IndexMap; /// During commitment operation, metadata indicates that operand tables cannot be the same. -#[derive(Debug, Error)] +#[derive(Debug, Snafu)] pub enum ColumnCommitmentsMismatch { /// Anonymous metadata indicates a column mismatch. - #[error(transparent)] - ColumnCommitmentMetadata(#[from] ColumnCommitmentMetadataMismatch), + #[snafu(transparent)] + ColumnCommitmentMetadata { + /// The underlying source error + source: ColumnCommitmentMetadataMismatch, + }, /// Commitments with different column counts cannot operate with each other. - #[error("commitments with different column counts cannot operate with each other")] + #[snafu(display("commitments with different column counts cannot operate with each other"))] NumColumns, /// Columns with mismatched identifiers cannot operate with each other. /// /// Strings are used here instead of Identifiers to decrease the size of this variant - #[error("column with identifier {0} cannot operate with column with identifier {1}")] - Identifier(String, String), + #[snafu(display( + "column with identifier {id_a} cannot operate with column with identifier {id_b}" + ))] + Identifier { + /// The first column identifier + id_a: String, + /// The second column identifier + id_b: String, + }, } /// Extension trait intended for [`ColumnCommitmentMetadataMap`]. @@ -90,10 +100,10 @@ impl ColumnCommitmentMetadataMapExt for ColumnCommitmentMetadataMap { .zip(other) .map(|((identifier_a, metadata_a), (identifier_b, metadata_b))| { if identifier_a != identifier_b { - Err(ColumnCommitmentsMismatch::Identifier( - identifier_a.to_string(), - identifier_b.to_string(), - ))? + Err(ColumnCommitmentsMismatch::Identifier { + id_a: identifier_a.to_string(), + id_b: identifier_b.to_string(), + })? } Ok((identifier_a, metadata_a.try_union(metadata_b)?)) @@ -113,10 +123,10 @@ impl ColumnCommitmentMetadataMapExt for ColumnCommitmentMetadataMap { .zip(other) .map(|((identifier_a, metadata_a), (identifier_b, metadata_b))| { if identifier_a != identifier_b { - Err(ColumnCommitmentsMismatch::Identifier( - identifier_a.to_string(), - identifier_b.to_string(), - ))? + Err(ColumnCommitmentsMismatch::Identifier { + id_a: identifier_a.to_string(), + id_b: identifier_b.to_string(), + })? } Ok((identifier_a, metadata_a.try_difference(metadata_b)?)) diff --git a/crates/proof-of-sql/src/base/commitment/column_commitments.rs b/crates/proof-of-sql/src/base/commitment/column_commitments.rs index 21633743a..3a2853439 100644 --- a/crates/proof-of-sql/src/base/commitment/column_commitments.rs +++ b/crates/proof-of-sql/src/base/commitment/column_commitments.rs @@ -13,22 +13,30 @@ use core::{iter, slice}; use indexmap::IndexSet; use proof_of_sql_parser::Identifier; use serde::{Deserialize, Serialize}; -use thiserror::Error; +use snafu::Snafu; /// Cannot create commitments with duplicate identifier. -#[derive(Debug, Error)] -#[error("cannot create commitments with duplicate identifier: {0}")] -pub struct DuplicateIdentifiers(String); +#[derive(Debug, Snafu)] +#[snafu(display("cannot create commitments with duplicate identifier: {id}"))] +pub struct DuplicateIdentifiers { + id: String, +} /// Errors that can occur when attempting to append rows to ColumnCommitments. -#[derive(Debug, Error)] +#[derive(Debug, Snafu)] pub enum AppendColumnCommitmentsError { /// Metadata between new and old columns are mismatched. - #[error(transparent)] - Mismatch(#[from] ColumnCommitmentsMismatch), + #[snafu(transparent)] + Mismatch { + /// The underlying source error + source: ColumnCommitmentsMismatch, + }, /// New columns have duplicate identifiers. - #[error(transparent)] - DuplicateIdentifiers(#[from] DuplicateIdentifiers), + #[snafu(transparent)] + DuplicateIdentifiers { + /// The underlying source error + source: DuplicateIdentifiers, + }, } /// Commitments for a collection of columns with some metadata. @@ -118,7 +126,9 @@ impl ColumnCommitments { if unique_identifiers.insert(identifier) { Ok((identifier, column)) } else { - Err(DuplicateIdentifiers(identifier.to_string())) + Err(DuplicateIdentifiers { + id: identifier.to_string(), + }) } }) .collect::, _>>()?; @@ -168,7 +178,9 @@ impl ColumnCommitments { if unique_identifiers.insert(identifier) { Ok((identifier, column)) } else { - Err(DuplicateIdentifiers(identifier.to_string())) + Err(DuplicateIdentifiers { + id: identifier.to_string(), + }) } }) .collect::, _>>()?; @@ -216,7 +228,9 @@ impl ColumnCommitments { .into_iter() .map(|(identifier, column)| { if self.column_metadata.contains_key(identifier) { - Err(DuplicateIdentifiers(identifier.to_string())) + Err(DuplicateIdentifiers { + id: identifier.to_string(), + }) } else { Ok((identifier, column)) } @@ -469,7 +483,10 @@ mod tests { 0, &(), ); - assert!(matches!(from_columns_result, Err(DuplicateIdentifiers(_)))); + assert!(matches!( + from_columns_result, + Err(DuplicateIdentifiers { .. }) + )); let mut existing_column_commitments = ColumnCommitments::::try_from_columns_with_offset( @@ -486,7 +503,7 @@ mod tests { .try_extend_columns_with_offset([(&duplicate_identifier_a, &empty_column)], 0, &()); assert!(matches!( extend_with_existing_column_result, - Err(DuplicateIdentifiers(_)) + Err(DuplicateIdentifiers { .. }) )); let extend_with_duplicate_columns_result = existing_column_commitments @@ -500,7 +517,7 @@ mod tests { ); assert!(matches!( extend_with_duplicate_columns_result, - Err(DuplicateIdentifiers(_)) + Err(DuplicateIdentifiers { .. }) )); let append_result = existing_column_commitments.try_append_rows_with_offset( @@ -514,7 +531,7 @@ mod tests { ); assert!(matches!( append_result, - Err(AppendColumnCommitmentsError::DuplicateIdentifiers(_)) + Err(AppendColumnCommitmentsError::DuplicateIdentifiers { .. }) )); } @@ -627,9 +644,9 @@ mod tests { ]); assert!(matches!( base_commitments.try_append_rows_with_offset(table_diff_type.inner_table(), 4, &()), - Err(AppendColumnCommitmentsError::Mismatch( - ColumnCommitmentsMismatch::ColumnCommitmentMetadata(_) - )) + Err(AppendColumnCommitmentsError::Mismatch { + source: ColumnCommitmentsMismatch::ColumnCommitmentMetadata { .. } + }) )); let table_diff_id: OwnedTable = owned_table([ @@ -642,18 +659,18 @@ mod tests { ); assert!(matches!( base_commitments.try_append_rows_with_offset(table_diff_id.inner_table(), 4, &()), - Err(AppendColumnCommitmentsError::Mismatch( - ColumnCommitmentsMismatch::Identifier(..) - )) + Err(AppendColumnCommitmentsError::Mismatch { + source: ColumnCommitmentsMismatch::Identifier { .. } + }) )); let table_diff_len: OwnedTable = owned_table([bigint("column_a", [5, 6, 7, 8])]); assert!(matches!( base_commitments.try_append_rows_with_offset(table_diff_len.inner_table(), 4, &()), - Err(AppendColumnCommitmentsError::Mismatch( - ColumnCommitmentsMismatch::NumColumns - )) + Err(AppendColumnCommitmentsError::Mismatch { + source: ColumnCommitmentsMismatch::NumColumns + }) )); } @@ -768,7 +785,7 @@ mod tests { .unwrap(); assert!(matches!( base_commitments.clone().try_add(commitments_diff_type), - Err(ColumnCommitmentsMismatch::ColumnCommitmentMetadata(_)) + Err(ColumnCommitmentsMismatch::ColumnCommitmentMetadata { .. }) )); let table_diff_id: OwnedTable = owned_table([ @@ -780,7 +797,7 @@ mod tests { .unwrap(); assert!(matches!( base_commitments.clone().try_add(commitments_diff_id), - Err(ColumnCommitmentsMismatch::Identifier(..)) + Err(ColumnCommitmentsMismatch::Identifier { .. }) )); let table_diff_len: OwnedTable = @@ -896,7 +913,7 @@ mod tests { .unwrap(); assert!(matches!( minuend_commitments.clone().try_sub(commitments_diff_type), - Err(ColumnCommitmentsMismatch::ColumnCommitmentMetadata(_)) + Err(ColumnCommitmentsMismatch::ColumnCommitmentMetadata { .. }) )); let table_diff_id: OwnedTable = @@ -906,7 +923,7 @@ mod tests { .unwrap(); assert!(matches!( minuend_commitments.clone().try_sub(commitments_diff_id), - Err(ColumnCommitmentsMismatch::Identifier(..)) + Err(ColumnCommitmentsMismatch::Identifier { .. }) )); let table_diff_len: OwnedTable = diff --git a/crates/proof-of-sql/src/base/commitment/table_commitment.rs b/crates/proof-of-sql/src/base/commitment/table_commitment.rs index dd6094ff8..1a8f9b769 100644 --- a/crates/proof-of-sql/src/base/commitment/table_commitment.rs +++ b/crates/proof-of-sql/src/base/commitment/table_commitment.rs @@ -15,76 +15,108 @@ use bumpalo::Bump; use core::ops::Range; use proof_of_sql_parser::{Identifier, ParseError}; use serde::{Deserialize, Serialize}; -use thiserror::Error; +use snafu::Snafu; /// Cannot create a [`TableCommitment`] with a negative range. -#[derive(Debug, Error)] -#[error("cannot create a TableCommitment with a negative range")] +#[derive(Debug, Snafu)] +#[snafu(display("cannot create a TableCommitment with a negative range"))] pub struct NegativeRange; /// Cannot create a [`TableCommitment`] from columns of mixed length. -#[derive(Debug, Error)] -#[error("cannot create a TableCommitment from columns of mixed length")] +#[derive(Debug, Snafu)] +#[snafu(display("cannot create a TableCommitment from columns of mixed length"))] pub struct MixedLengthColumns; /// Errors that can occur when trying to create or extend a [`TableCommitment`] from columns. -#[derive(Debug, Error)] +#[derive(Debug, Snafu)] pub enum TableCommitmentFromColumnsError { /// Cannot construct [`TableCommitment`] from columns of mixed length. - #[error(transparent)] - MixedLengthColumns(#[from] MixedLengthColumns), + #[snafu(transparent)] + MixedLengthColumns { + /// The underlying source error + source: MixedLengthColumns, + }, /// Cannot construct [`TableCommitment`] from columns with duplicate identifiers. - #[error(transparent)] - DuplicateIdentifiers(#[from] DuplicateIdentifiers), + #[snafu(transparent)] + DuplicateIdentifiers { + /// The underlying source error + source: DuplicateIdentifiers, + }, } /// Errors that can occur when attempting to append rows to a [`TableCommitment`]. -#[derive(Debug, Error)] +#[derive(Debug, Snafu)] pub enum AppendTableCommitmentError { /// Cannot append columns of mixed length to existing [`TableCommitment`]. - #[error(transparent)] - MixedLengthColumns(#[from] MixedLengthColumns), + #[snafu(transparent)] + MixedLengthColumns { + /// The underlying source error + source: MixedLengthColumns, + }, /// Encountered error when appending internal [`ColumnCommitments`]. - #[error(transparent)] - AppendColumnCommitments(#[from] AppendColumnCommitmentsError), + #[snafu(transparent)] + AppendColumnCommitments { + /// The underlying source error + source: AppendColumnCommitmentsError, + }, } /// Errors that can occur when performing arithmetic on [`TableCommitment`]s. -#[derive(Debug, Error)] +#[derive(Debug, Snafu)] pub enum TableCommitmentArithmeticError { /// Cannot perform arithmetic on columns with mismatched metadata. - #[error(transparent)] - ColumnMismatch(#[from] ColumnCommitmentsMismatch), + #[snafu(transparent)] + ColumnMismatch { + /// The underlying source error + source: ColumnCommitmentsMismatch, + }, /// Cannot perform TableCommitment arithmetic that would result in a negative range. - #[error(transparent)] - NegativeRange(#[from] NegativeRange), + #[snafu(transparent)] + NegativeRange { + /// The underlying source error + source: NegativeRange, + }, /// Cannot perform arithmetic for noncontiguous table commitments. - #[error("cannot perform table commitment arithmetic for noncontiguous table commitments")] + #[snafu(display( + "cannot perform table commitment arithmetic for noncontiguous table commitments" + ))] NonContiguous, } /// Errors that can occur when trying to create or extend a [`TableCommitment`] from a record batch. #[cfg(feature = "arrow")] -#[derive(Debug, Error)] +#[derive(Debug, Snafu)] pub enum RecordBatchToColumnsError { /// Error converting from arrow array - #[error(transparent)] - ArrowArrayToColumnConversionError(#[from] ArrowArrayToColumnConversionError), - #[error(transparent)] + #[snafu(transparent)] + ArrowArrayToColumnConversionError { + /// The underlying source error + source: ArrowArrayToColumnConversionError, + }, + #[snafu(transparent)] /// This error occurs when convering from a record batch name to an identifier fails. (Which may be impossible.) - FieldParseFail(#[from] ParseError), + FieldParseFail { + /// The underlying source error + source: ParseError, + }, } /// Errors that can occur when attempting to append a record batch to a [`TableCommitment`]. #[cfg(feature = "arrow")] -#[derive(Debug, Error)] +#[derive(Debug, Snafu)] pub enum AppendRecordBatchTableCommitmentError { /// During commitment operation, metadata indicates that operand tables cannot be the same. - #[error(transparent)] - ColumnCommitmentsMismatch(#[from] ColumnCommitmentsMismatch), + #[snafu(transparent)] + ColumnCommitmentsMismatch { + /// The underlying source error + source: ColumnCommitmentsMismatch, + }, /// Error converting from arrow array - #[error(transparent)] - ArrowBatchToColumnError(#[from] RecordBatchToColumnsError), + #[snafu(transparent)] + ArrowBatchToColumnError { + /// The underlying source error + source: RecordBatchToColumnsError, + }, } /// Commitment for an entire table, with column and table metadata. @@ -246,13 +278,13 @@ impl TableCommitment { { self.try_append_rows(owned_table.inner_table(), setup) .map_err(|e| match e { - AppendTableCommitmentError::AppendColumnCommitments(e) => match e { - AppendColumnCommitmentsError::Mismatch(e) => e, - AppendColumnCommitmentsError::DuplicateIdentifiers(_) => { + AppendTableCommitmentError::AppendColumnCommitments { source: e } => match e { + AppendColumnCommitmentsError::Mismatch { source: e } => e, + AppendColumnCommitmentsError::DuplicateIdentifiers { .. } => { panic!("OwnedTables cannot have duplicate identifiers"); } }, - AppendTableCommitmentError::MixedLengthColumns(_) => { + AppendTableCommitmentError::MixedLengthColumns { .. } => { panic!("OwnedTables cannot have columns of mixed length"); } }) @@ -370,17 +402,17 @@ impl TableCommitment { setup, ) { Ok(()) => Ok(()), - Err(AppendTableCommitmentError::MixedLengthColumns(_)) => { + Err(AppendTableCommitmentError::MixedLengthColumns { .. }) => { panic!("RecordBatches cannot have columns of mixed length") } - Err(AppendTableCommitmentError::AppendColumnCommitments( - AppendColumnCommitmentsError::DuplicateIdentifiers(_), - )) => { + Err(AppendTableCommitmentError::AppendColumnCommitments { + source: AppendColumnCommitmentsError::DuplicateIdentifiers { .. }, + }) => { panic!("RecordBatches cannot have duplicate identifiers") } - Err(AppendTableCommitmentError::AppendColumnCommitments( - AppendColumnCommitmentsError::Mismatch(e), - )) => Err(e)?, + Err(AppendTableCommitmentError::AppendColumnCommitments { + source: AppendColumnCommitmentsError::Mismatch { source: e }, + }) => Err(e)?, } } /// Returns a [`TableCommitment`] to the provided arrow [`RecordBatch`]. @@ -407,10 +439,10 @@ impl TableCommitment { setup, ) { Ok(commitment) => Ok(commitment), - Err(TableCommitmentFromColumnsError::MixedLengthColumns(_)) => { + Err(TableCommitmentFromColumnsError::MixedLengthColumns { .. }) => { panic!("RecordBatches cannot have columns of mixed length") } - Err(TableCommitmentFromColumnsError::DuplicateIdentifiers(_)) => { + Err(TableCommitmentFromColumnsError::DuplicateIdentifiers { .. }) => { panic!("RecordBatches cannot have duplicate identifiers") } } @@ -560,7 +592,7 @@ mod tests { ); assert!(matches!( from_columns_result, - Err(TableCommitmentFromColumnsError::DuplicateIdentifiers(_)) + Err(TableCommitmentFromColumnsError::DuplicateIdentifiers { .. }) )); let mut table_commitment = TableCommitment::::try_from_columns_with_offset( @@ -578,7 +610,7 @@ mod tests { table_commitment.try_extend_columns([(&duplicate_identifier_a, &empty_column)], &()); assert!(matches!( extend_columns_result, - Err(TableCommitmentFromColumnsError::DuplicateIdentifiers(_)) + Err(TableCommitmentFromColumnsError::DuplicateIdentifiers { .. }) )); let extend_columns_result = table_commitment.try_extend_columns( @@ -590,7 +622,7 @@ mod tests { ); assert!(matches!( extend_columns_result, - Err(TableCommitmentFromColumnsError::DuplicateIdentifiers(_)) + Err(TableCommitmentFromColumnsError::DuplicateIdentifiers { .. }) )); // make sure the commitment wasn't mutated @@ -617,7 +649,7 @@ mod tests { ); assert!(matches!( from_columns_result, - Err(TableCommitmentFromColumnsError::MixedLengthColumns(_)) + Err(TableCommitmentFromColumnsError::MixedLengthColumns { .. }) )); let mut table_commitment = TableCommitment::::try_from_columns_with_offset( @@ -632,7 +664,7 @@ mod tests { table_commitment.try_extend_columns([(&column_id_b, &two_row_column)], &()); assert!(matches!( extend_columns_result, - Err(TableCommitmentFromColumnsError::MixedLengthColumns(_)) + Err(TableCommitmentFromColumnsError::MixedLengthColumns { .. }) )); let extend_columns_result = table_commitment.try_extend_columns( @@ -644,7 +676,7 @@ mod tests { ); assert!(matches!( extend_columns_result, - Err(TableCommitmentFromColumnsError::MixedLengthColumns(_)) + Err(TableCommitmentFromColumnsError::MixedLengthColumns { .. }) )); // make sure the commitment wasn't mutated @@ -726,11 +758,11 @@ mod tests { ]); assert!(matches!( table_commitment.try_append_rows(table_diff_type.inner_table(), &()), - Err(AppendTableCommitmentError::AppendColumnCommitments( - AppendColumnCommitmentsError::Mismatch( - ColumnCommitmentsMismatch::ColumnCommitmentMetadata(_) - ) - )) + Err(AppendTableCommitmentError::AppendColumnCommitments { + source: AppendColumnCommitmentsError::Mismatch { + source: ColumnCommitmentsMismatch::ColumnCommitmentMetadata { .. } + } + }) )); // make sure the commitment wasn't mutated @@ -763,9 +795,9 @@ mod tests { ); assert!(matches!( append_column_result, - Err(AppendTableCommitmentError::AppendColumnCommitments( - AppendColumnCommitmentsError::DuplicateIdentifiers(_) - )) + Err(AppendTableCommitmentError::AppendColumnCommitments { + source: AppendColumnCommitmentsError::DuplicateIdentifiers { .. } + }) )); // make sure the commitment wasn't mutated @@ -803,7 +835,7 @@ mod tests { ); assert!(matches!( append_result, - Err(AppendTableCommitmentError::MixedLengthColumns(_)) + Err(AppendTableCommitmentError::MixedLengthColumns { .. }) )); // make sure the commitment wasn't mutated @@ -927,7 +959,7 @@ mod tests { .unwrap(); assert!(matches!( table_commitment.try_add(table_commitment_diff_type), - Err(TableCommitmentArithmeticError::ColumnMismatch(_)) + Err(TableCommitmentArithmeticError::ColumnMismatch { .. }) )); } @@ -1080,7 +1112,7 @@ mod tests { .unwrap(); assert!(matches!( table_commitment.try_sub(table_commitment_diff_type), - Err(TableCommitmentArithmeticError::ColumnMismatch(_)) + Err(TableCommitmentArithmeticError::ColumnMismatch { .. }) )); } @@ -1202,7 +1234,7 @@ mod tests { table_commitment_low.try_sub(table_commitment_all.clone()); assert!(matches!( try_negative_high_difference_result, - Err(TableCommitmentArithmeticError::NegativeRange(_)) + Err(TableCommitmentArithmeticError::NegativeRange { .. }) )); // try to subtract the total commitment off the high to get the "negative" low commitment @@ -1210,7 +1242,7 @@ mod tests { table_commitment_high.try_sub(table_commitment_all); assert!(matches!( try_negative_low_difference_result, - Err(TableCommitmentArithmeticError::NegativeRange(_)) + Err(TableCommitmentArithmeticError::NegativeRange { .. }) )); } diff --git a/crates/proof-of-sql/src/base/commitment/vec_commitment_ext.rs b/crates/proof-of-sql/src/base/commitment/vec_commitment_ext.rs index 167649625..695cb08dd 100644 --- a/crates/proof-of-sql/src/base/commitment/vec_commitment_ext.rs +++ b/crates/proof-of-sql/src/base/commitment/vec_commitment_ext.rs @@ -1,11 +1,11 @@ use super::Commitment; use crate::base::commitment::committable_column::CommittableColumn; use alloc::{vec, vec::Vec}; -use thiserror::Error; +use snafu::Snafu; /// Cannot update commitment collections with different column counts -#[derive(Error, Debug)] -#[error("cannot update commitment collections with different column counts")] +#[derive(Snafu, Debug)] +#[snafu(display("cannot update commitment collections with different column counts"))] pub struct NumColumnsMismatch; /// Extension trait intended for collections of commitments. diff --git a/crates/proof-of-sql/src/base/database/arrow_array_to_column_conversion.rs b/crates/proof-of-sql/src/base/database/arrow_array_to_column_conversion.rs index 77a6bfc46..c5ac9e9c5 100644 --- a/crates/proof-of-sql/src/base/database/arrow_array_to_column_conversion.rs +++ b/crates/proof-of-sql/src/base/database/arrow_array_to_column_conversion.rs @@ -10,30 +10,49 @@ use arrow::{ }; use bumpalo::Bump; use proof_of_sql_parser::posql_time::{PoSQLTimeUnit, PoSQLTimeZone, PoSQLTimestampError}; +use snafu::Snafu; use std::ops::Range; -use thiserror::Error; -#[derive(Error, Debug, PartialEq)] +#[derive(Snafu, Debug, PartialEq)] /// Errors caused by conversions between Arrow and owned types. pub enum ArrowArrayToColumnConversionError { /// This error occurs when an array contains a non-zero number of null elements - #[error("arrow array must not contain nulls")] + #[snafu(display("arrow array must not contain nulls"))] ArrayContainsNulls, /// This error occurs when trying to convert from an unsupported arrow type. - #[error("unsupported type: attempted conversion from ArrayRef of type {0} to OwnedColumn")] - UnsupportedType(DataType), + #[snafu(display( + "unsupported type: attempted conversion from ArrayRef of type {datatype} to OwnedColumn" + ))] + UnsupportedType { + /// The unsupported datatype + datatype: DataType, + }, /// Variant for decimal errors - #[error(transparent)] - DecimalError(#[from] crate::base::math::decimal::DecimalError), + #[snafu(transparent)] + DecimalError { + /// The underlying source error + source: crate::base::math::decimal::DecimalError, + }, /// This error occurs when trying to convert from an i256 to a Scalar. - #[error("decimal conversion failed: {0}")] - DecimalConversionFailed(i256), + #[snafu(display("decimal conversion failed: {number}"))] + DecimalConversionFailed { + /// The `i256` value for which conversion is attempted + number: i256, + }, /// This error occurs when the specified range is out of the bounds of the array. - #[error("index out of bounds: the len is {0} but the index is {1}")] - IndexOutOfBounds(usize, usize), + #[snafu(display("index out of bounds: the len is {len} but the index is {index}"))] + IndexOutOfBounds { + /// The actual length of the array + len: usize, + /// The out of bounds index requested + index: usize, + }, /// Using TimeError to handle all time-related errors - #[error(transparent)] - TimestampConversionError(#[from] PoSQLTimestampError), + #[snafu(transparent)] + TimestampConversionError { + /// The underlying source error + source: PoSQLTimestampError, + }, } /// This trait is used to provide utility functions to convert ArrayRefs into proof types (Column, Scalars, etc.) @@ -114,7 +133,9 @@ impl ArrayRefExt for ArrayRef { .iter() .map(|v| { convert_i256_to_scalar(v).ok_or( - ArrowArrayToColumnConversionError::DecimalConversionFailed(*v), + ArrowArrayToColumnConversionError::DecimalConversionFailed { + number: *v, + }, ) }) .collect() @@ -151,9 +172,9 @@ impl ArrayRefExt for ArrayRef { }; result.unwrap_or_else(|| { - Err(ArrowArrayToColumnConversionError::UnsupportedType( - self.data_type().clone(), - )) + Err(ArrowArrayToColumnConversionError::UnsupportedType { + datatype: self.data_type().clone(), + }) }) } @@ -188,10 +209,10 @@ impl ArrayRefExt for ArrayRef { // Before performing any operations, check if the range is out of bounds if range.end > self.len() { - return Err(ArrowArrayToColumnConversionError::IndexOutOfBounds( - self.len(), - range.end, - )); + return Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { + len: self.len(), + index: range.end, + }); } // Match supported types and attempt conversion match self.data_type() { @@ -206,45 +227,45 @@ impl ArrayRefExt for ArrayRef { let values = alloc.alloc_slice_fill_with(range.len(), |i| boolean_slice[i]); Ok(Column::Boolean(values)) } else { - Err(ArrowArrayToColumnConversionError::UnsupportedType( - self.data_type().clone(), - )) + Err(ArrowArrayToColumnConversionError::UnsupportedType { + datatype: self.data_type().clone(), + }) } } DataType::Int16 => { if let Some(array) = self.as_any().downcast_ref::() { Ok(Column::SmallInt(&array.values()[range.start..range.end])) } else { - Err(ArrowArrayToColumnConversionError::UnsupportedType( - self.data_type().clone(), - )) + Err(ArrowArrayToColumnConversionError::UnsupportedType { + datatype: self.data_type().clone(), + }) } } DataType::Int32 => { if let Some(array) = self.as_any().downcast_ref::() { Ok(Column::Int(&array.values()[range.start..range.end])) } else { - Err(ArrowArrayToColumnConversionError::UnsupportedType( - self.data_type().clone(), - )) + Err(ArrowArrayToColumnConversionError::UnsupportedType { + datatype: self.data_type().clone(), + }) } } DataType::Int64 => { if let Some(array) = self.as_any().downcast_ref::() { Ok(Column::BigInt(&array.values()[range.start..range.end])) } else { - Err(ArrowArrayToColumnConversionError::UnsupportedType( - self.data_type().clone(), - )) + Err(ArrowArrayToColumnConversionError::UnsupportedType { + datatype: self.data_type().clone(), + }) } } DataType::Decimal128(38, 0) => { if let Some(array) = self.as_any().downcast_ref::() { Ok(Column::Int128(&array.values()[range.start..range.end])) } else { - Err(ArrowArrayToColumnConversionError::UnsupportedType( - self.data_type().clone(), - )) + Err(ArrowArrayToColumnConversionError::UnsupportedType { + datatype: self.data_type().clone(), + }) } } DataType::Decimal256(precision, scale) if *precision <= 75 => { @@ -253,7 +274,9 @@ impl ArrayRefExt for ArrayRef { let scalars = alloc.alloc_slice_fill_default(i256_slice.len()); for (scalar, value) in scalars.iter_mut().zip(i256_slice) { *scalar = convert_i256_to_scalar(value).ok_or( - ArrowArrayToColumnConversionError::DecimalConversionFailed(*value), + ArrowArrayToColumnConversionError::DecimalConversionFailed { + number: *value, + }, )?; } Ok(Column::Decimal75( @@ -262,9 +285,9 @@ impl ArrayRefExt for ArrayRef { scalars, )) } else { - Err(ArrowArrayToColumnConversionError::UnsupportedType( - self.data_type().clone(), - )) + Err(ArrowArrayToColumnConversionError::UnsupportedType { + datatype: self.data_type().clone(), + }) } } // Handle all possible TimeStamp TimeUnit instances @@ -277,9 +300,9 @@ impl ArrayRefExt for ArrayRef { &array.values()[range.start..range.end], )) } else { - Err(ArrowArrayToColumnConversionError::UnsupportedType( - self.data_type().clone(), - )) + Err(ArrowArrayToColumnConversionError::UnsupportedType { + datatype: self.data_type().clone(), + }) } } ArrowTimeUnit::Millisecond => { @@ -290,9 +313,9 @@ impl ArrayRefExt for ArrayRef { &array.values()[range.start..range.end], )) } else { - Err(ArrowArrayToColumnConversionError::UnsupportedType( - self.data_type().clone(), - )) + Err(ArrowArrayToColumnConversionError::UnsupportedType { + datatype: self.data_type().clone(), + }) } } ArrowTimeUnit::Microsecond => { @@ -303,9 +326,9 @@ impl ArrayRefExt for ArrayRef { &array.values()[range.start..range.end], )) } else { - Err(ArrowArrayToColumnConversionError::UnsupportedType( - self.data_type().clone(), - )) + Err(ArrowArrayToColumnConversionError::UnsupportedType { + datatype: self.data_type().clone(), + }) } } ArrowTimeUnit::Nanosecond => { @@ -316,9 +339,9 @@ impl ArrayRefExt for ArrayRef { &array.values()[range.start..range.end], )) } else { - Err(ArrowArrayToColumnConversionError::UnsupportedType( - self.data_type().clone(), - )) + Err(ArrowArrayToColumnConversionError::UnsupportedType { + datatype: self.data_type().clone(), + }) } } }, @@ -337,14 +360,14 @@ impl ArrayRefExt for ArrayRef { Ok(Column::VarChar((vals, scals))) } else { - Err(ArrowArrayToColumnConversionError::UnsupportedType( - self.data_type().clone(), - )) + Err(ArrowArrayToColumnConversionError::UnsupportedType { + datatype: self.data_type().clone(), + }) } } - data_type => Err(ArrowArrayToColumnConversionError::UnsupportedType( - data_type.clone(), - )), + data_type => Err(ArrowArrayToColumnConversionError::UnsupportedType { + datatype: data_type.clone(), + }), } } } @@ -420,7 +443,7 @@ mod tests { let result = array.to_column::(&alloc, &(3..5), None); assert_eq!( result, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(3, 5)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 3, index: 5 }) ); } @@ -447,7 +470,7 @@ mod tests { let result = array.to_column::(&alloc, &(2..4), None); assert_eq!( result, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(3, 4)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 3, index: 4 }) ); } @@ -570,7 +593,7 @@ mod tests { let result = array.to_column::(&alloc, &(2..4), None); assert_eq!( result, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(3, 4)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 3, index: 4 }) ); } @@ -617,7 +640,7 @@ mod tests { let result = array.to_column::(&alloc, &(2..4), None); assert_eq!( result, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(3, 4)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 3, index: 4 }) ); } @@ -689,7 +712,7 @@ mod tests { assert_eq!( result, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(3, 4)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 3, index: 4 }) ); } @@ -729,7 +752,7 @@ mod tests { assert_eq!( result, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(3, 4)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 3, index: 4 }) ); } @@ -769,7 +792,7 @@ mod tests { assert_eq!( result, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(3, 4)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 3, index: 4 }) ); } @@ -792,21 +815,21 @@ mod tests { let result1 = array1.to_column::(&alloc, &(2..3), None); assert_eq!( result1, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(2, 3)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 2, index: 3 }) ); let array2: ArrayRef = Arc::new(arrow::array::Int32Array::from(vec![1, -3])); let result2 = array2.to_column::(&alloc, &(2..3), None); assert_eq!( result2, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(2, 3)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 2, index: 3 }) ); let array3: ArrayRef = Arc::new(arrow::array::Int64Array::from(vec![1, -3])); let result3 = array3.to_column::(&alloc, &(2..3), None); assert_eq!( result3, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(2, 3)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 2, index: 3 }) ); } @@ -818,21 +841,21 @@ mod tests { let result1 = array1.to_column::(&alloc, &(5..5), None); assert_eq!( result1, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(2, 5)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 2, index: 5 }) ); let array2: ArrayRef = Arc::new(arrow::array::Int32Array::from(vec![1, -3])); let result2 = array2.to_column::(&alloc, &(5..5), None); assert_eq!( result2, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(2, 5)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 2, index: 5 }) ); let array3: ArrayRef = Arc::new(arrow::array::Int64Array::from(vec![1, -3])); let result3 = array3.to_column::(&alloc, &(5..5), None); assert_eq!( result3, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(2, 5)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 2, index: 5 }) ); } @@ -924,7 +947,7 @@ mod tests { let result = array.to_column::(&alloc, &(0..3), None); assert_eq!( result, - Err(ArrowArrayToColumnConversionError::IndexOutOfBounds(2, 3)) + Err(ArrowArrayToColumnConversionError::IndexOutOfBounds { len: 2, index: 3 }) ); } diff --git a/crates/proof-of-sql/src/base/database/column_operation.rs b/crates/proof-of-sql/src/base/database/column_operation.rs index 94a6e69d3..078306b2c 100644 --- a/crates/proof-of-sql/src/base/database/column_operation.rs +++ b/crates/proof-of-sql/src/base/database/column_operation.rs @@ -50,16 +50,16 @@ pub fn try_add_subtract_column_types( .max(right_precision_value - right_scale as i16) + 1_i16; let precision = u8::try_from(precision_value) - .map_err(|_| { - ColumnOperationError::DecimalConversionError(DecimalError::InvalidPrecision( - precision_value.to_string(), - )) + .map_err(|_| ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidPrecision { + error: precision_value.to_string(), + }, }) .and_then(|p| { - Precision::new(p).map_err(|_| { - ColumnOperationError::DecimalConversionError(DecimalError::InvalidPrecision( - p.to_string(), - )) + Precision::new(p).map_err(|_| ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidPrecision { + error: p.to_string(), + }, }) })?; Ok(ColumnType::Decimal75(precision, scale)) @@ -91,17 +91,23 @@ pub fn try_multiply_column_types( let right_precision_value = rhs.precision_value().expect("Numeric types have precision"); let precision_value = left_precision_value + right_precision_value + 1; let precision = Precision::new(precision_value).map_err(|_| { - ColumnOperationError::DecimalConversionError(DecimalError::InvalidPrecision(format!( - "Required precision {} is beyond what we can support", - precision_value - ))) + ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidPrecision { + error: format!( + "Required precision {} is beyond what we can support", + precision_value + ), + }, + } })?; let left_scale = lhs.scale().expect("Numeric types have scale"); let right_scale = rhs.scale().expect("Numeric types have scale"); let scale = left_scale.checked_add(right_scale).ok_or( - ColumnOperationError::DecimalConversionError(DecimalError::InvalidScale( - left_scale as i16 + right_scale as i16, - )), + ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidScale { + scale: left_scale as i16 + right_scale as i16, + }, + }, )?; Ok(ColumnType::Decimal75(precision, scale)) } @@ -135,20 +141,21 @@ pub fn try_divide_column_types( let right_scale = rhs.scale().expect("Numeric types have scale") as i16; let raw_scale = (left_scale + right_precision_value + 1_i16).max(6_i16); let precision_value: i16 = left_precision_value - left_scale + right_scale + raw_scale; - let scale = i8::try_from(raw_scale).map_err(|_| { - ColumnOperationError::DecimalConversionError(DecimalError::InvalidScale(raw_scale)) - })?; + let scale = + i8::try_from(raw_scale).map_err(|_| ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidScale { scale: raw_scale }, + })?; let precision = u8::try_from(precision_value) - .map_err(|_| { - ColumnOperationError::DecimalConversionError(DecimalError::InvalidPrecision( - precision_value.to_string(), - )) + .map_err(|_| ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidPrecision { + error: precision_value.to_string(), + }, }) .and_then(|p| { - Precision::new(p).map_err(|_| { - ColumnOperationError::DecimalConversionError(DecimalError::InvalidPrecision( - p.to_string(), - )) + Precision::new(p).map_err(|_| ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidPrecision { + error: p.to_string(), + }, }) })?; Ok(ColumnType::Decimal75(precision, scale)) @@ -233,10 +240,9 @@ where .zip(rhs.iter()) .map(|(l, r)| -> ColumnOperationResult { l.checked_add(r) - .ok_or(ColumnOperationError::IntegerOverflow(format!( - "Overflow in integer addition {:?} + {:?}", - l, r - ))) + .ok_or(ColumnOperationError::IntegerOverflow { + error: format!("Overflow in integer addition {:?} + {:?}", l, r), + }) }) .collect::>>() } @@ -252,10 +258,9 @@ where .zip(rhs.iter()) .map(|(l, r)| -> ColumnOperationResult { l.checked_sub(r) - .ok_or(ColumnOperationError::IntegerOverflow(format!( - "Overflow in integer subtraction {:?} - {:?}", - l, r - ))) + .ok_or(ColumnOperationError::IntegerOverflow { + error: format!("Overflow in integer subtraction {:?} - {:?}", l, r), + }) }) .collect::>>() } @@ -271,10 +276,9 @@ where .zip(rhs.iter()) .map(|(l, r)| -> ColumnOperationResult { l.checked_mul(r) - .ok_or(ColumnOperationError::IntegerOverflow(format!( - "Overflow in integer multiplication {:?} * {:?}", - l, r - ))) + .ok_or(ColumnOperationError::IntegerOverflow { + error: format!("Overflow in integer multiplication {:?} * {:?}", l, r), + }) }) .collect::>>() } @@ -369,10 +373,9 @@ where .zip(numbers_of_larger_type.iter()) .map(|(l, r)| -> ColumnOperationResult { Into::::into(*l).checked_add(r).ok_or( - ColumnOperationError::IntegerOverflow(format!( - "Overflow in integer addition {:?} + {:?}", - l, r - )), + ColumnOperationError::IntegerOverflow { + error: format!("Overflow in integer addition {:?} + {:?}", l, r), + }, ) }) .collect() @@ -393,10 +396,9 @@ where .zip(rhs.iter()) .map(|(l, r)| -> ColumnOperationResult { Into::::into(*l).checked_sub(r).ok_or( - ColumnOperationError::IntegerOverflow(format!( - "Overflow in integer subtraction {:?} - {:?}", - l, r - )), + ColumnOperationError::IntegerOverflow { + error: format!("Overflow in integer subtraction {:?} - {:?}", l, r), + }, ) }) .collect() @@ -417,10 +419,9 @@ where .zip(rhs.iter()) .map(|(l, r)| -> ColumnOperationResult { l.checked_sub(&Into::::into(*r)).ok_or( - ColumnOperationError::IntegerOverflow(format!( - "Overflow in integer subtraction {:?} - {:?}", - l, r - )), + ColumnOperationError::IntegerOverflow { + error: format!("Overflow in integer subtraction {:?} - {:?}", l, r), + }, ) }) .collect() @@ -442,10 +443,9 @@ where .zip(numbers_of_larger_type.iter()) .map(|(l, r)| -> ColumnOperationResult { Into::::into(*l).checked_mul(r).ok_or( - ColumnOperationError::IntegerOverflow(format!( - "Overflow in integer multiplication {:?} * {:?}", - l, r - )), + ColumnOperationError::IntegerOverflow { + error: format!("Overflow in integer multiplication {:?} * {:?}", l, r), + }, ) }) .collect() @@ -978,18 +978,18 @@ mod test { let rhs = ColumnType::Decimal75(Precision::new(73).unwrap(), 4); assert!(matches!( try_add_subtract_column_types(lhs, rhs, BinaryOperator::Add), - Err(ColumnOperationError::DecimalConversionError( - DecimalError::InvalidPrecision(_) - )) + Err(ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidPrecision { .. } + }) )); let lhs = ColumnType::Int; let rhs = ColumnType::Decimal75(Precision::new(75).unwrap(), 10); assert!(matches!( try_add_subtract_column_types(lhs, rhs, BinaryOperator::Add), - Err(ColumnOperationError::DecimalConversionError( - DecimalError::InvalidPrecision(_) - )) + Err(ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidPrecision { .. } + }) )); } @@ -1076,18 +1076,18 @@ mod test { let rhs = ColumnType::Decimal75(Precision::new(73).unwrap(), 1); assert!(matches!( try_add_subtract_column_types(lhs, rhs, BinaryOperator::Subtract), - Err(ColumnOperationError::DecimalConversionError( - DecimalError::InvalidPrecision(_) - )) + Err(ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidPrecision { .. } + }) )); let lhs = ColumnType::Int128; let rhs = ColumnType::Decimal75(Precision::new(75).unwrap(), 12); assert!(matches!( try_add_subtract_column_types(lhs, rhs, BinaryOperator::Subtract), - Err(ColumnOperationError::DecimalConversionError( - DecimalError::InvalidPrecision(_) - )) + Err(ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidPrecision { .. } + }) )); } @@ -1175,18 +1175,18 @@ mod test { let rhs = ColumnType::Decimal75(Precision::new(37).unwrap(), 4); assert!(matches!( try_multiply_column_types(lhs, rhs), - Err(ColumnOperationError::DecimalConversionError( - DecimalError::InvalidPrecision(_) - )) + Err(ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidPrecision { .. } + }) )); let lhs = ColumnType::Int; let rhs = ColumnType::Decimal75(Precision::new(65).unwrap(), 0); assert!(matches!( try_multiply_column_types(lhs, rhs), - Err(ColumnOperationError::DecimalConversionError( - DecimalError::InvalidPrecision(_) - )) + Err(ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidPrecision { .. } + }) )); // Invalid scale @@ -1194,18 +1194,18 @@ mod test { let rhs = ColumnType::Decimal75(Precision::new(5).unwrap(), -65_i8); assert!(matches!( try_multiply_column_types(lhs, rhs), - Err(ColumnOperationError::DecimalConversionError( - DecimalError::InvalidScale(_) - )) + Err(ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidScale { .. } + }) )); let lhs = ColumnType::Decimal75(Precision::new(5).unwrap(), 64_i8); let rhs = ColumnType::Decimal75(Precision::new(5).unwrap(), 64_i8); assert!(matches!( try_multiply_column_types(lhs, rhs), - Err(ColumnOperationError::DecimalConversionError( - DecimalError::InvalidScale(_) - )) + Err(ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidScale { .. } + }) )); } @@ -1300,18 +1300,18 @@ mod test { let rhs = ColumnType::Decimal75(Precision::new(13).unwrap(), -14); assert!(matches!( try_divide_column_types(lhs, rhs), - Err(ColumnOperationError::DecimalConversionError( - DecimalError::InvalidPrecision(_) - )) + Err(ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidPrecision { .. } + }) )); let lhs = ColumnType::Int; let rhs = ColumnType::Decimal75(Precision::new(68).unwrap(), 67); assert!(matches!( try_divide_column_types(lhs, rhs), - Err(ColumnOperationError::DecimalConversionError( - DecimalError::InvalidPrecision(_) - )) + Err(ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidPrecision { .. } + }) )); // Invalid scale @@ -1319,9 +1319,9 @@ mod test { let rhs = ColumnType::Decimal75(Precision::new(75).unwrap(), 40_i8); assert!(matches!( try_divide_column_types(lhs, rhs), - Err(ColumnOperationError::DecimalConversionError( - DecimalError::InvalidScale(_) - )) + Err(ColumnOperationError::DecimalConversionError { + source: DecimalError::InvalidScale { .. } + }) )); } @@ -1740,7 +1740,7 @@ mod test { let rhs = [1_i16, 1]; assert!(matches!( try_add_slices(&lhs, &rhs), - Err(ColumnOperationError::IntegerOverflow(_)) + Err(ColumnOperationError::IntegerOverflow { .. }) )); } @@ -1759,7 +1759,7 @@ mod test { let rhs = [i32::MIN, 1]; assert!(matches!( try_add_slices_with_casting(&lhs, &rhs), - Err(ColumnOperationError::IntegerOverflow(_)) + Err(ColumnOperationError::IntegerOverflow { .. }) )); } @@ -1883,7 +1883,7 @@ mod test { let rhs = [1_i128, 1]; assert!(matches!( try_subtract_slices(&lhs, &rhs), - Err(ColumnOperationError::IntegerOverflow(_)) + Err(ColumnOperationError::IntegerOverflow { .. }) )); } @@ -1902,7 +1902,7 @@ mod test { let rhs = [i32::MIN, 1]; assert!(matches!( try_subtract_slices_left_upcast(&lhs, &rhs), - Err(ColumnOperationError::IntegerOverflow(_)) + Err(ColumnOperationError::IntegerOverflow { .. }) )); } @@ -1921,7 +1921,7 @@ mod test { let rhs = [1_i16, 1]; assert!(matches!( try_subtract_slices_right_upcast(&lhs, &rhs), - Err(ColumnOperationError::IntegerOverflow(_)) + Err(ColumnOperationError::IntegerOverflow { .. }) )); } @@ -2045,7 +2045,7 @@ mod test { let rhs = [2, 2]; assert!(matches!( try_multiply_slices(&lhs, &rhs), - Err(ColumnOperationError::IntegerOverflow(_)) + Err(ColumnOperationError::IntegerOverflow { .. }) )); } @@ -2064,7 +2064,7 @@ mod test { let rhs = [i32::MAX, 2]; assert!(matches!( try_multiply_slices_with_casting(&lhs, &rhs), - Err(ColumnOperationError::IntegerOverflow(_)) + Err(ColumnOperationError::IntegerOverflow { .. }) )); } diff --git a/crates/proof-of-sql/src/base/database/column_operation_error.rs b/crates/proof-of-sql/src/base/database/column_operation_error.rs index fc0e6634c..e67876772 100644 --- a/crates/proof-of-sql/src/base/database/column_operation_error.rs +++ b/crates/proof-of-sql/src/base/database/column_operation_error.rs @@ -1,16 +1,21 @@ use crate::base::{database::ColumnType, math::decimal::DecimalError}; use proof_of_sql_parser::intermediate_ast::{BinaryOperator, UnaryOperator}; -use thiserror::Error; +use snafu::Snafu; /// Errors from operations on columns. -#[derive(Error, Debug, PartialEq, Eq)] +#[derive(Snafu, Debug, PartialEq, Eq)] pub enum ColumnOperationError { /// Two columns do not have the same length - #[error("Columns have different lengths: {0} != {1}")] - DifferentColumnLength(usize, usize), + #[snafu(display("Columns have different lengths: {len_a} != {len_b}"))] + DifferentColumnLength { + /// The length of the first column + len_a: usize, + /// The length of the second column + len_b: usize, + }, /// Incorrect `ColumnType` in binary operations - #[error("{operator:?}(lhs: {left_type:?}, rhs: {right_type:?}) is not supported")] + #[snafu(display("{operator:?}(lhs: {left_type:?}, rhs: {right_type:?}) is not supported"))] BinaryOperationInvalidColumnType { /// `BinaryOperator` that caused the error operator: BinaryOperator, @@ -21,7 +26,7 @@ pub enum ColumnOperationError { }, /// Incorrect `ColumnType` in unary operations - #[error("{operator:?}(operand: {operand_type:?}) is not supported")] + #[snafu(display("{operator:?}(operand: {operand_type:?}) is not supported"))] UnaryOperationInvalidColumnType { /// `UnaryOperator` that caused the error operator: UnaryOperator, @@ -30,16 +35,22 @@ pub enum ColumnOperationError { }, /// Overflow in integer operations - #[error("Overflow in integer operation: {0}")] - IntegerOverflow(String), + #[snafu(display("Overflow in integer operation: {error}"))] + IntegerOverflow { + /// The underlying overflow error + error: String, + }, /// Division by zero - #[error("Division by zero")] + #[snafu(display("Division by zero"))] DivisionByZero, /// Errors related to decimal operations - #[error(transparent)] - DecimalConversionError(#[from] DecimalError), + #[snafu(transparent)] + DecimalConversionError { + /// The underlying source error + source: DecimalError, + }, } /// Result type for column operations diff --git a/crates/proof-of-sql/src/base/database/expression_evaluation.rs b/crates/proof-of-sql/src/base/database/expression_evaluation.rs index a8121ba3a..a37270223 100644 --- a/crates/proof-of-sql/src/base/database/expression_evaluation.rs +++ b/crates/proof-of-sql/src/base/database/expression_evaluation.rs @@ -17,10 +17,9 @@ impl OwnedTable { Expression::Literal(lit) => self.evaluate_literal(lit), Expression::Binary { op, left, right } => self.evaluate_binary_expr(*op, left, right), Expression::Unary { op, expr } => self.evaluate_unary_expr(*op, expr), - _ => Err(ExpressionEvaluationError::Unsupported(format!( - "Expression {:?} is not supported yet", - expr - ))), + _ => Err(ExpressionEvaluationError::Unsupported { + expression: format!("Expression {:?} is not supported yet", expr), + }), } } @@ -31,9 +30,9 @@ impl OwnedTable { Ok(self .inner_table() .get(identifier) - .ok_or(ExpressionEvaluationError::ColumnNotFound( - identifier.to_string(), - ))? + .ok_or(ExpressionEvaluationError::ColumnNotFound { + error: identifier.to_string(), + })? .clone()) } diff --git a/crates/proof-of-sql/src/base/database/expression_evaluation_error.rs b/crates/proof-of-sql/src/base/database/expression_evaluation_error.rs index 696dfe6ec..dee94576e 100644 --- a/crates/proof-of-sql/src/base/database/expression_evaluation_error.rs +++ b/crates/proof-of-sql/src/base/database/expression_evaluation_error.rs @@ -1,21 +1,33 @@ use crate::base::{database::ColumnOperationError, math::decimal::DecimalError}; -use thiserror::Error; +use snafu::Snafu; /// Errors from evaluation of `Expression`s. -#[derive(Error, Debug, PartialEq, Eq)] +#[derive(Snafu, Debug, PartialEq, Eq)] pub enum ExpressionEvaluationError { /// Column not found - #[error("Column not found: {0}")] - ColumnNotFound(String), + #[snafu(display("Column not found: {error}"))] + ColumnNotFound { + /// The underlying error + error: String, + }, /// Error in column operation - #[error(transparent)] - ColumnOperationError(#[from] ColumnOperationError), + #[snafu(transparent)] + ColumnOperationError { + /// The underlying source error + source: ColumnOperationError, + }, /// Expression not yet supported - #[error("Expression {0} is not supported yet")] - Unsupported(String), + #[snafu(display("Expression {expression} is not supported yet"))] + Unsupported { + /// The unsupported expression + expression: String, + }, /// Error in decimal conversion - #[error(transparent)] - DecimalConversionError(#[from] DecimalError), + #[snafu(transparent)] + DecimalConversionError { + /// The underlying source error + source: DecimalError, + }, } /// Result type for expression evaluation diff --git a/crates/proof-of-sql/src/base/database/expression_evaluation_test.rs b/crates/proof-of-sql/src/base/database/expression_evaluation_test.rs index e58a56e57..9bb2ddaaf 100644 --- a/crates/proof-of-sql/src/base/database/expression_evaluation_test.rs +++ b/crates/proof-of-sql/src/base/database/expression_evaluation_test.rs @@ -83,7 +83,7 @@ fn we_can_not_evaluate_a_nonexisting_column() { let expr = col("not_a_column"); assert!(matches!( table.evaluate(&expr), - Err(ExpressionEvaluationError::ColumnNotFound(_)) + Err(ExpressionEvaluationError::ColumnNotFound { .. }) )); } @@ -207,44 +207,44 @@ fn we_cannot_evaluate_expressions_if_column_operation_errors_out() { let expr = not(col("language")); assert!(matches!( table.evaluate(&expr), - Err(ExpressionEvaluationError::ColumnOperationError( - ColumnOperationError::UnaryOperationInvalidColumnType { .. } - )) + Err(ExpressionEvaluationError::ColumnOperationError { + source: ColumnOperationError::UnaryOperationInvalidColumnType { .. } + }) )); // NOT doesn't work on bigint let expr = not(col("bigints")); assert!(matches!( table.evaluate(&expr), - Err(ExpressionEvaluationError::ColumnOperationError( - ColumnOperationError::UnaryOperationInvalidColumnType { .. } - )) + Err(ExpressionEvaluationError::ColumnOperationError { + source: ColumnOperationError::UnaryOperationInvalidColumnType { .. } + }) )); // + doesn't work on varchar let expr = add(col("sarah"), col("bigints")); assert!(matches!( table.evaluate(&expr), - Err(ExpressionEvaluationError::ColumnOperationError( - ColumnOperationError::BinaryOperationInvalidColumnType { .. } - )) + Err(ExpressionEvaluationError::ColumnOperationError { + source: ColumnOperationError::BinaryOperationInvalidColumnType { .. } + }) )); // i64::MIN - 1 overflows let expr = sub(col("bigints"), lit(1)); assert!(matches!( table.evaluate(&expr), - Err(ExpressionEvaluationError::ColumnOperationError( - ColumnOperationError::IntegerOverflow(_) - )) + Err(ExpressionEvaluationError::ColumnOperationError { + source: ColumnOperationError::IntegerOverflow { .. } + }) )); // We can't divide by zero let expr = div(col("bigints"), lit(0)); assert!(matches!( table.evaluate(&expr), - Err(ExpressionEvaluationError::ColumnOperationError( - ColumnOperationError::DivisionByZero - )) + Err(ExpressionEvaluationError::ColumnOperationError { + source: ColumnOperationError::DivisionByZero + }) )); } diff --git a/crates/proof-of-sql/src/base/database/group_by_util.rs b/crates/proof-of-sql/src/base/database/group_by_util.rs index ed5f00616..0d0f8c571 100644 --- a/crates/proof-of-sql/src/base/database/group_by_util.rs +++ b/crates/proof-of-sql/src/base/database/group_by_util.rs @@ -8,7 +8,7 @@ use bumpalo::Bump; use core::cmp::Ordering; use itertools::Itertools; use rayon::prelude::ParallelSliceMut; -use thiserror::Error; +use snafu::Snafu; /// The output of the `aggregate_columns` function. #[derive(Debug)] @@ -27,9 +27,9 @@ pub struct AggregatedColumns<'a, S: Scalar> { /// The number of rows in each group. pub count_column: &'a [i64], } -#[derive(Error, Debug, PartialEq, Eq)] +#[derive(Snafu, Debug, PartialEq, Eq)] pub enum AggregateColumnsError { - #[error("Column length mismatch")] + #[snafu(display("Column length mismatch"))] ColumnLengthMismatch, } diff --git a/crates/proof-of-sql/src/base/database/owned_and_arrow_conversions.rs b/crates/proof-of-sql/src/base/database/owned_and_arrow_conversions.rs index 76c41339d..59d03f749 100644 --- a/crates/proof-of-sql/src/base/database/owned_and_arrow_conversions.rs +++ b/crates/proof-of-sql/src/base/database/owned_and_arrow_conversions.rs @@ -36,31 +36,45 @@ use proof_of_sql_parser::{ posql_time::{PoSQLTimeUnit, PoSQLTimeZone, PoSQLTimestampError}, Identifier, ParseError, }; +use snafu::Snafu; use std::sync::Arc; -use thiserror::Error; -#[derive(Error, Debug)] +#[derive(Snafu, Debug)] #[non_exhaustive] /// Errors cause by conversions between Arrow and owned types. pub enum OwnedArrowConversionError { /// This error occurs when trying to convert from an unsupported arrow type. - #[error("unsupported type: attempted conversion from ArrayRef of type {0} to OwnedColumn")] - UnsupportedType(DataType), + #[snafu(display( + "unsupported type: attempted conversion from ArrayRef of type {datatype} to OwnedColumn" + ))] + UnsupportedType { + /// The unsupported datatype + datatype: DataType, + }, /// This error occurs when trying to convert from a record batch with duplicate identifiers (e.g. `"a"` and `"A"`). - #[error("conversion resulted in duplicate identifiers")] + #[snafu(display("conversion resulted in duplicate identifiers"))] DuplicateIdentifiers, - #[error(transparent)] /// This error occurs when convering from a record batch name to an identifier fails. (Which may my impossible.) - FieldParseFail(#[from] ParseError), - #[error(transparent)] + #[snafu(transparent)] + FieldParseFail { + /// The underlying source error + source: ParseError, + }, /// This error occurs when creating an owned table fails, which should only occur when there are zero columns. - InvalidTable(#[from] OwnedTableError), + #[snafu(transparent)] + InvalidTable { + /// The underlying source error + source: OwnedTableError, + }, /// This error occurs when trying to convert from an Arrow array with nulls. - #[error("null values are not supported in OwnedColumn yet")] + #[snafu(display("null values are not supported in OwnedColumn yet"))] NullNotSupportedYet, /// Using TimeError to handle all time-related errors - #[error(transparent)] - TimestampConversionError(#[from] PoSQLTimestampError), + #[snafu(transparent)] + TimestampConversionError { + /// The underlying source error + source: PoSQLTimestampError, + }, } impl From> for ArrayRef { @@ -245,9 +259,9 @@ impl TryFrom<&ArrayRef> for OwnedColumn { )) } }, - &data_type => Err(OwnedArrowConversionError::UnsupportedType( - data_type.clone(), - )), + &data_type => Err(OwnedArrowConversionError::UnsupportedType { + datatype: data_type.clone(), + }), } } } diff --git a/crates/proof-of-sql/src/base/database/owned_and_arrow_conversions_test.rs b/crates/proof-of-sql/src/base/database/owned_and_arrow_conversions_test.rs index e1f75fb91..4c7287bbb 100644 --- a/crates/proof-of-sql/src/base/database/owned_and_arrow_conversions_test.rs +++ b/crates/proof-of-sql/src/base/database/owned_and_arrow_conversions_test.rs @@ -76,7 +76,7 @@ fn we_get_an_unsupported_type_error_when_trying_to_convert_from_a_float32_array_ let array_ref: ArrayRef = Arc::new(Float32Array::from(vec![0.0])); assert!(matches!( OwnedColumn::::try_from(array_ref), - Err(OwnedArrowConversionError::UnsupportedType(_)) + Err(OwnedArrowConversionError::UnsupportedType { .. }) )); } diff --git a/crates/proof-of-sql/src/base/database/owned_column.rs b/crates/proof-of-sql/src/base/database/owned_column.rs index f4d8b7700..674bd3d6f 100644 --- a/crates/proof-of-sql/src/base/database/owned_column.rs +++ b/crates/proof-of-sql/src/base/database/owned_column.rs @@ -137,10 +137,8 @@ impl OwnedColumn { .iter() .map(|s| -> Result { TryInto::::try_into(*s) }) .collect::, _>>() - .map_err(|_| { - OwnedColumnError::ScalarConversionError( - "Overflow in scalar conversions".to_string(), - ) + .map_err(|_| OwnedColumnError::ScalarConversionError { + error: "Overflow in scalar conversions".to_string(), })?, )), ColumnType::SmallInt => Ok(OwnedColumn::SmallInt( @@ -148,10 +146,8 @@ impl OwnedColumn { .iter() .map(|s| -> Result { TryInto::::try_into(*s) }) .collect::, _>>() - .map_err(|_| { - OwnedColumnError::ScalarConversionError( - "Overflow in scalar conversions".to_string(), - ) + .map_err(|_| OwnedColumnError::ScalarConversionError { + error: "Overflow in scalar conversions".to_string(), })?, )), ColumnType::Int => Ok(OwnedColumn::Int( @@ -159,10 +155,8 @@ impl OwnedColumn { .iter() .map(|s| -> Result { TryInto::::try_into(*s) }) .collect::, _>>() - .map_err(|_| { - OwnedColumnError::ScalarConversionError( - "Overflow in scalar conversions".to_string(), - ) + .map_err(|_| OwnedColumnError::ScalarConversionError { + error: "Overflow in scalar conversions".to_string(), })?, )), ColumnType::BigInt => Ok(OwnedColumn::BigInt( @@ -170,10 +164,8 @@ impl OwnedColumn { .iter() .map(|s| -> Result { TryInto::::try_into(*s) }) .collect::, _>>() - .map_err(|_| { - OwnedColumnError::ScalarConversionError( - "Overflow in scalar conversions".to_string(), - ) + .map_err(|_| OwnedColumnError::ScalarConversionError { + error: "Overflow in scalar conversions".to_string(), })?, )), ColumnType::Int128 => Ok(OwnedColumn::Int128( @@ -181,10 +173,8 @@ impl OwnedColumn { .iter() .map(|s| -> Result { TryInto::::try_into(*s) }) .collect::, _>>() - .map_err(|_| { - OwnedColumnError::ScalarConversionError( - "Overflow in scalar conversions".to_string(), - ) + .map_err(|_| OwnedColumnError::ScalarConversionError { + error: "Overflow in scalar conversions".to_string(), })?, )), ColumnType::Scalar => Ok(OwnedColumn::Scalar(scalars.to_vec())), @@ -196,10 +186,8 @@ impl OwnedColumn { .iter() .map(|s| -> Result { TryInto::::try_into(*s) }) .collect::, _>>() - .map_err(|_| { - OwnedColumnError::ScalarConversionError( - "Overflow in scalar conversions".to_string(), - ) + .map_err(|_| OwnedColumnError::ScalarConversionError { + error: "Overflow in scalar conversions".to_string(), })?; Ok(OwnedColumn::TimestampTZ(tu, tz, raw_values)) } @@ -220,9 +208,9 @@ impl OwnedColumn { .iter() .copied() .collect::>>() - .ok_or(OwnedColumnError::Unsupported( - "NULL is not supported yet".to_string(), - ))?; + .ok_or(OwnedColumnError::Unsupported { + error: "NULL is not supported yet".to_string(), + })?; Self::try_from_scalars(&scalars, column_type) } @@ -523,7 +511,7 @@ mod test { let res = OwnedColumn::try_from_scalars(&scalars, column_type); assert!(matches!( res, - Err(OwnedColumnError::ScalarConversionError(_)) + Err(OwnedColumnError::ScalarConversionError { .. }) )); // Boolean @@ -535,7 +523,7 @@ mod test { let res = OwnedColumn::try_from_scalars(&scalars, column_type); assert!(matches!( res, - Err(OwnedColumnError::ScalarConversionError(_)) + Err(OwnedColumnError::ScalarConversionError { .. }) )); } @@ -607,7 +595,7 @@ mod test { let res = OwnedColumn::try_from_option_scalars(&option_scalars, column_type); assert!(matches!( res, - Err(OwnedColumnError::ScalarConversionError(_)) + Err(OwnedColumnError::ScalarConversionError { .. }) )); // Boolean @@ -625,7 +613,7 @@ mod test { let res = OwnedColumn::try_from_option_scalars(&option_scalars, column_type); assert!(matches!( res, - Err(OwnedColumnError::ScalarConversionError(_)) + Err(OwnedColumnError::ScalarConversionError { .. }) )); } @@ -638,7 +626,7 @@ mod test { .collect::>(); let column_type = ColumnType::Int128; let res = OwnedColumn::try_from_option_scalars(&option_scalars, column_type); - assert!(matches!(res, Err(OwnedColumnError::Unsupported(_)))); + assert!(matches!(res, Err(OwnedColumnError::Unsupported { .. }))); // Boolean let option_scalars = [Some(true), Some(false), None, Some(false), Some(true)] @@ -647,6 +635,6 @@ mod test { .collect::>(); let column_type = ColumnType::Boolean; let res = OwnedColumn::try_from_option_scalars(&option_scalars, column_type); - assert!(matches!(res, Err(OwnedColumnError::Unsupported(_)))); + assert!(matches!(res, Err(OwnedColumnError::Unsupported { .. }))); } } diff --git a/crates/proof-of-sql/src/base/database/owned_column_error.rs b/crates/proof-of-sql/src/base/database/owned_column_error.rs index e00c8e717..84509d758 100644 --- a/crates/proof-of-sql/src/base/database/owned_column_error.rs +++ b/crates/proof-of-sql/src/base/database/owned_column_error.rs @@ -1,24 +1,30 @@ use crate::base::database::ColumnType; use alloc::string::String; -use thiserror::Error; +use snafu::Snafu; /// Errors from operations related to `OwnedColumn`s. -#[derive(Error, Debug, PartialEq, Eq)] +#[derive(Snafu, Debug, PartialEq, Eq)] pub enum OwnedColumnError { /// Can not perform type casting. - #[error("Can not perform type casting from {from_type:?} to {to_type:?}")] + #[snafu(display("Can not perform type casting from {from_type:?} to {to_type:?}"))] TypeCastError { /// The type from which we are trying to cast. from_type: ColumnType, /// The type to which we are trying to cast. to_type: ColumnType, }, - /// Error in converting scalars to a given column type. - #[error("Error in converting scalars to a given column type: {0}")] - ScalarConversionError(String), + /// Error in converting scalars to a given column type. + #[snafu(display("Error in converting scalars to a given column type: {error}"))] + ScalarConversionError { + /// The underlying error + error: String, + }, /// Unsupported operation. - #[error("Unsupported operation: {0}")] - Unsupported(String), + #[snafu(display("Unsupported operation: {error}"))] + Unsupported { + /// The underlying error + error: String, + }, } /// Result type for operations related to `OwnedColumn`s. diff --git a/crates/proof-of-sql/src/base/database/owned_column_operation.rs b/crates/proof-of-sql/src/base/database/owned_column_operation.rs index 98163bfaf..345c959ed 100644 --- a/crates/proof-of-sql/src/base/database/owned_column_operation.rs +++ b/crates/proof-of-sql/src/base/database/owned_column_operation.rs @@ -21,10 +21,10 @@ impl OwnedColumn { /// Element-wise AND for two columns pub fn element_wise_and(&self, rhs: &Self) -> ColumnOperationResult { if self.len() != rhs.len() { - return Err(ColumnOperationError::DifferentColumnLength( - self.len(), - rhs.len(), - )); + return Err(ColumnOperationError::DifferentColumnLength { + len_a: self.len(), + len_b: rhs.len(), + }); } match (self, rhs) { (Self::Boolean(lhs), Self::Boolean(rhs)) => Ok(Self::Boolean(slice_and(lhs, rhs))), @@ -39,10 +39,10 @@ impl OwnedColumn { /// Element-wise OR for two columns pub fn element_wise_or(&self, rhs: &Self) -> ColumnOperationResult { if self.len() != rhs.len() { - return Err(ColumnOperationError::DifferentColumnLength( - self.len(), - rhs.len(), - )); + return Err(ColumnOperationError::DifferentColumnLength { + len_a: self.len(), + len_b: rhs.len(), + }); } match (self, rhs) { (Self::Boolean(lhs), Self::Boolean(rhs)) => Ok(Self::Boolean(slice_or(lhs, rhs))), @@ -57,10 +57,10 @@ impl OwnedColumn { /// Element-wise equality check for two columns pub fn element_wise_eq(&self, rhs: &Self) -> ColumnOperationResult { if self.len() != rhs.len() { - return Err(ColumnOperationError::DifferentColumnLength( - self.len(), - rhs.len(), - )); + return Err(ColumnOperationError::DifferentColumnLength { + len_a: self.len(), + len_b: rhs.len(), + }); } match (self, rhs) { (Self::SmallInt(lhs), Self::SmallInt(rhs)) => Ok(Self::Boolean(slice_eq(lhs, rhs))), @@ -192,10 +192,10 @@ impl OwnedColumn { /// Element-wise <= check for two columns pub fn element_wise_le(&self, rhs: &Self) -> ColumnOperationResult { if self.len() != rhs.len() { - return Err(ColumnOperationError::DifferentColumnLength( - self.len(), - rhs.len(), - )); + return Err(ColumnOperationError::DifferentColumnLength { + len_a: self.len(), + len_b: rhs.len(), + }); } match (self, rhs) { (Self::SmallInt(lhs), Self::SmallInt(rhs)) => Ok(Self::Boolean(slice_le(lhs, rhs))), @@ -326,10 +326,10 @@ impl OwnedColumn { /// Element-wise >= check for two columns pub fn element_wise_ge(&self, rhs: &Self) -> ColumnOperationResult { if self.len() != rhs.len() { - return Err(ColumnOperationError::DifferentColumnLength( - self.len(), - rhs.len(), - )); + return Err(ColumnOperationError::DifferentColumnLength { + len_a: self.len(), + len_b: rhs.len(), + }); } match (self, rhs) { (Self::SmallInt(lhs), Self::SmallInt(rhs)) => Ok(Self::Boolean(slice_ge(lhs, rhs))), @@ -463,10 +463,10 @@ impl Add for OwnedColumn { fn add(self, rhs: Self) -> Self::Output { if self.len() != rhs.len() { - return Err(ColumnOperationError::DifferentColumnLength( - self.len(), - rhs.len(), - )); + return Err(ColumnOperationError::DifferentColumnLength { + len_a: self.len(), + len_b: rhs.len(), + }); } match (&self, &rhs) { (Self::SmallInt(lhs), Self::SmallInt(rhs)) => { @@ -606,10 +606,10 @@ impl Sub for OwnedColumn { fn sub(self, rhs: Self) -> Self::Output { if self.len() != rhs.len() { - return Err(ColumnOperationError::DifferentColumnLength( - self.len(), - rhs.len(), - )); + return Err(ColumnOperationError::DifferentColumnLength { + len_a: self.len(), + len_b: rhs.len(), + }); } match (&self, &rhs) { (Self::SmallInt(lhs), Self::SmallInt(rhs)) => { @@ -753,10 +753,10 @@ impl Mul for OwnedColumn { fn mul(self, rhs: Self) -> Self::Output { if self.len() != rhs.len() { - return Err(ColumnOperationError::DifferentColumnLength( - self.len(), - rhs.len(), - )); + return Err(ColumnOperationError::DifferentColumnLength { + len_a: self.len(), + len_b: rhs.len(), + }); } match (&self, &rhs) { (Self::SmallInt(lhs), Self::SmallInt(rhs)) => { @@ -900,10 +900,10 @@ impl Div for OwnedColumn { fn div(self, rhs: Self) -> Self::Output { if self.len() != rhs.len() { - return Err(ColumnOperationError::DifferentColumnLength( - self.len(), - rhs.len(), - )); + return Err(ColumnOperationError::DifferentColumnLength { + len_a: self.len(), + len_b: rhs.len(), + }); } match (&self, &rhs) { (Self::SmallInt(lhs), Self::SmallInt(rhs)) => { @@ -1055,25 +1055,25 @@ mod test { let result = lhs.element_wise_and(&rhs); assert!(matches!( result, - Err(ColumnOperationError::DifferentColumnLength(_, _)) + Err(ColumnOperationError::DifferentColumnLength { .. }) )); let result = lhs.element_wise_eq(&rhs); assert!(matches!( result, - Err(ColumnOperationError::DifferentColumnLength(_, _)) + Err(ColumnOperationError::DifferentColumnLength { .. }) )); let result = lhs.element_wise_le(&rhs); assert!(matches!( result, - Err(ColumnOperationError::DifferentColumnLength(_, _)) + Err(ColumnOperationError::DifferentColumnLength { .. }) )); let result = lhs.element_wise_ge(&rhs); assert!(matches!( result, - Err(ColumnOperationError::DifferentColumnLength(_, _)) + Err(ColumnOperationError::DifferentColumnLength { .. }) )); let lhs = OwnedColumn::::SmallInt(vec![1, 2, 3]); @@ -1081,25 +1081,25 @@ mod test { let result = lhs.clone() + rhs.clone(); assert!(matches!( result, - Err(ColumnOperationError::DifferentColumnLength(_, _)) + Err(ColumnOperationError::DifferentColumnLength { .. }) )); let result = lhs.clone() - rhs.clone(); assert!(matches!( result, - Err(ColumnOperationError::DifferentColumnLength(_, _)) + Err(ColumnOperationError::DifferentColumnLength { .. }) )); let result = lhs.clone() * rhs.clone(); assert!(matches!( result, - Err(ColumnOperationError::DifferentColumnLength(_, _)) + Err(ColumnOperationError::DifferentColumnLength { .. }) )); let result = lhs / rhs; assert!(matches!( result, - Err(ColumnOperationError::DifferentColumnLength(_, _)) + Err(ColumnOperationError::DifferentColumnLength { .. }) )); } diff --git a/crates/proof-of-sql/src/base/database/owned_table.rs b/crates/proof-of-sql/src/base/database/owned_table.rs index 6fdfc0662..5a6ce036d 100644 --- a/crates/proof-of-sql/src/base/database/owned_table.rs +++ b/crates/proof-of-sql/src/base/database/owned_table.rs @@ -2,13 +2,13 @@ use super::OwnedColumn; use crate::base::scalar::Scalar; use indexmap::IndexMap; use proof_of_sql_parser::Identifier; -use thiserror::Error; +use snafu::Snafu; /// An error that occurs when working with tables. -#[derive(Error, Debug, PartialEq, Eq)] +#[derive(Snafu, Debug, PartialEq, Eq)] pub enum OwnedTableError { /// The columns have different lengths. - #[error("Columns have different lengths")] + #[snafu(display("Columns have different lengths"))] ColumnLengthMismatch, } /// A table of data, with schema included. This is simply a map from `Identifier` to `OwnedColumn`, diff --git a/crates/proof-of-sql/src/base/math/decimal.rs b/crates/proof-of-sql/src/base/math/decimal.rs index c56211d86..991bf5ace 100644 --- a/crates/proof-of-sql/src/base/math/decimal.rs +++ b/crates/proof-of-sql/src/base/math/decimal.rs @@ -6,37 +6,52 @@ use alloc::{ }; use proof_of_sql_parser::intermediate_decimal::{IntermediateDecimal, IntermediateDecimalError}; use serde::{Deserialize, Deserializer, Serialize}; -use thiserror::Error; +use snafu::Snafu; /// Errors related to decimal operations. -#[derive(Error, Debug, Eq, PartialEq)] +#[derive(Snafu, Debug, Eq, PartialEq)] pub enum DecimalError { - #[error("Invalid decimal format or value: {0}")] + #[snafu(display("Invalid decimal format or value: {error}"))] /// Error when a decimal format or value is incorrect, /// the string isn't even a decimal e.g. "notastring", /// "-21.233.122" etc aka InvalidDecimal - InvalidDecimal(String), + InvalidDecimal { + /// The underlying error + error: String, + }, - #[error("Decimal precision is not valid: {0}")] + #[snafu(display("Decimal precision is not valid: {error}"))] /// Decimal precision exceeds the allowed limit, /// e.g. precision above 75/76/whatever set by Scalar /// or non-positive aka InvalidPrecision - InvalidPrecision(String), + InvalidPrecision { + /// The underlying error + error: String, + }, - #[error("Decimal scale is not valid: {0}")] + #[snafu(display("Decimal scale is not valid: {scale}"))] /// Decimal scale is not valid. Here we use i16 in order to include /// invalid scale values - InvalidScale(i16), + InvalidScale { + /// The invalid scale value + scale: i16, + }, - #[error("Unsupported operation: cannot round decimal: {0}")] + #[snafu(display("Unsupported operation: cannot round decimal: {error}"))] /// This error occurs when attempting to scale a /// decimal in such a way that a loss of precision occurs. - RoundingError(String), + RoundingError { + /// The underlying error + error: String, + }, /// Errors that may occur when parsing an intermediate decimal /// into a posql decimal - #[error(transparent)] - IntermediateDecimalConversionError(#[from] IntermediateDecimalError), + #[snafu(transparent)] + IntermediateDecimalConversionError { + /// The underlying source error + source: IntermediateDecimalError, + }, } /// Result type for decimal operations. @@ -58,10 +73,12 @@ impl Precision { /// Constructor for creating a Precision instance pub fn new(value: u8) -> Result { if value > MAX_SUPPORTED_PRECISION || value == 0 { - Err(DecimalError::InvalidPrecision(format!( - "Failed to parse precision. Value of {} exceeds max supported precision of {}", - value, MAX_SUPPORTED_PRECISION - ))) + Err(DecimalError::InvalidPrecision { + error: format!( + "Failed to parse precision. Value of {} exceeds max supported precision of {}", + value, MAX_SUPPORTED_PRECISION + ), + }) } else { Ok(Precision(value)) } @@ -116,9 +133,9 @@ impl Decimal { ) -> DecimalResult> { let scale_factor = new_scale - self.scale; if scale_factor < 0 || new_precision.value() < self.precision.value() + scale_factor as u8 { - return Err(DecimalError::RoundingError( - "Scale factor must be non-negative".to_string(), - )); + return Err(DecimalError::RoundingError { + error: "Scale factor must be non-negative".to_string(), + }); } let scaled_value = scale_scalar(self.value, scale_factor)?; Ok(Decimal::new(scaled_value, new_precision, new_scale)) @@ -129,14 +146,14 @@ impl Decimal { const MINIMAL_PRECISION: u8 = 19; let raw_precision = precision.value(); if raw_precision < MINIMAL_PRECISION { - return Err(DecimalError::RoundingError( - "Precision must be at least 19".to_string(), - )); + return Err(DecimalError::RoundingError { + error: "Precision must be at least 19".to_string(), + }); } if scale < 0 || raw_precision < MINIMAL_PRECISION + scale as u8 { - return Err(DecimalError::RoundingError( - "Can not scale down a decimal".to_string(), - )); + return Err(DecimalError::RoundingError { + error: "Can not scale down a decimal".to_string(), + }); } let scaled_value = scale_scalar(S::from(&value), scale)?; Ok(Decimal::new(scaled_value, precision, scale)) @@ -147,14 +164,14 @@ impl Decimal { const MINIMAL_PRECISION: u8 = 39; let raw_precision = precision.value(); if raw_precision < MINIMAL_PRECISION { - return Err(DecimalError::RoundingError( - "Precision must be at least 19".to_string(), - )); + return Err(DecimalError::RoundingError { + error: "Precision must be at least 19".to_string(), + }); } if scale < 0 || raw_precision < MINIMAL_PRECISION + scale as u8 { - return Err(DecimalError::RoundingError( - "Can not scale down a decimal".to_string(), - )); + return Err(DecimalError::RoundingError { + error: "Can not scale down a decimal".to_string(), + }); } let scaled_value = scale_scalar(S::from(&value), scale)?; Ok(Decimal::new(scaled_value, precision, scale)) @@ -185,7 +202,9 @@ pub(crate) fn try_into_to_scalar( ) -> DecimalResult { d.try_into_bigint_with_precision_and_scale(target_precision.value(), target_scale)? .try_into() - .map_err(|e: ScalarConversionError| DecimalError::InvalidDecimal(e.to_string())) + .map_err(|e: ScalarConversionError| DecimalError::InvalidDecimal { + error: e.to_string(), + }) } /// Scale scalar by the given scale factor. Negative scaling is not allowed. @@ -193,9 +212,9 @@ pub(crate) fn try_into_to_scalar( pub(crate) fn scale_scalar(s: S, scale: i8) -> DecimalResult { match scale { 0 => Ok(s), - _ if scale < 0 => Err(DecimalError::RoundingError( - "Scale factor must be non-negative".to_string(), - )), + _ if scale < 0 => Err(DecimalError::RoundingError { + error: "Scale factor must be non-negative".to_string(), + }), _ => { let ten = S::from(10); let mut res = s; diff --git a/crates/proof-of-sql/src/base/math/permutation.rs b/crates/proof-of-sql/src/base/math/permutation.rs index 4f90e578d..e629b36d3 100644 --- a/crates/proof-of-sql/src/base/math/permutation.rs +++ b/crates/proof-of-sql/src/base/math/permutation.rs @@ -1,14 +1,14 @@ use alloc::{format, string::String, vec::Vec}; -use thiserror::Error; +use snafu::Snafu; /// An error that occurs when working with permutations -#[derive(Error, Debug, PartialEq, Eq)] +#[derive(Snafu, Debug, PartialEq, Eq)] pub enum PermutationError { /// The permutation is invalid - #[error("Permutation is invalid {0}")] - InvalidPermutation(String), + #[snafu(display("Permutation is invalid {error}"))] + InvalidPermutation { error: String }, /// Application of a permutation to a slice with an incorrect length - #[error("Application of a permutation to a slice with a different length {permutation_size} != {slice_length}")] + #[snafu(display("Application of a permutation to a slice with a different length {permutation_size} != {slice_length}"))] PermutationSizeMismatch { permutation_size: usize, slice_length: usize, @@ -39,17 +39,21 @@ impl Permutation { elements.sort_unstable(); elements.dedup(); if elements.len() < length { - Err(PermutationError::InvalidPermutation(format!( - "Permutation can not have duplicate elements: {:?}", - permutation - ))) + Err(PermutationError::InvalidPermutation { + error: format!( + "Permutation can not have duplicate elements: {:?}", + permutation + ), + }) } // Check that no element is out of bounds else if permutation.iter().any(|&i| i >= length) { - Err(PermutationError::InvalidPermutation(format!( - "Permutation can not have elements out of bounds: {:?}", - permutation - ))) + Err(PermutationError::InvalidPermutation { + error: format!( + "Permutation can not have elements out of bounds: {:?}", + permutation + ), + }) } else { Ok(Self { permutation }) } @@ -95,11 +99,11 @@ mod test { fn test_invalid_permutation() { assert!(matches!( Permutation::try_new(vec![1, 0, 0]), - Err(PermutationError::InvalidPermutation(_)) + Err(PermutationError::InvalidPermutation { .. }) )); assert!(matches!( Permutation::try_new(vec![1, 0, 3]), - Err(PermutationError::InvalidPermutation(_)) + Err(PermutationError::InvalidPermutation { .. }) )); } diff --git a/crates/proof-of-sql/src/base/proof/error.rs b/crates/proof-of-sql/src/base/proof/error.rs index 95a407d18..5d675060a 100644 --- a/crates/proof-of-sql/src/base/proof/error.rs +++ b/crates/proof-of-sql/src/base/proof/error.rs @@ -1,9 +1,9 @@ -use thiserror::Error; +use snafu::Snafu; -#[derive(Error, Debug)] +#[derive(Snafu, Debug)] /// These errors occur when a proof failed to verify. pub enum ProofError { - #[error("Verification error: {0}")] + #[snafu(display("Verification error: {error}"))] /// This error occurs when a proof failed to verify. - VerificationError(&'static str), + VerificationError { error: &'static str }, } diff --git a/crates/proof-of-sql/src/base/scalar/error.rs b/crates/proof-of-sql/src/base/scalar/error.rs index ebda7f89a..c012a1d87 100644 --- a/crates/proof-of-sql/src/base/scalar/error.rs +++ b/crates/proof-of-sql/src/base/scalar/error.rs @@ -1,10 +1,13 @@ use alloc::string::String; -use thiserror::Error; +use snafu::Snafu; -#[derive(Error, Debug)] +#[derive(Snafu, Debug)] /// These errors occur when a scalar conversion fails. pub enum ScalarConversionError { - #[error("Overflow error: {0}")] + #[snafu(display("Overflow error: {error}"))] /// This error occurs when a scalar is too large to be converted. - Overflow(String), + Overflow { + /// The underlying error + error: String, + }, } diff --git a/crates/proof-of-sql/src/base/scalar/mont_scalar.rs b/crates/proof-of-sql/src/base/scalar/mont_scalar.rs index 21a830130..81c264e9c 100644 --- a/crates/proof-of-sql/src/base/scalar/mont_scalar.rs +++ b/crates/proof-of-sql/src/base/scalar/mont_scalar.rs @@ -165,11 +165,11 @@ impl> TryFrom for MontScalar { // Check if the number of digits exceeds the maximum precision allowed if digits.len() > MAX_SUPPORTED_PRECISION.into() { - return Err(ScalarConversionError::Overflow(format!( + return Err(ScalarConversionError::Overflow{ error: format!( "Attempted to parse a number with {} digits, which exceeds the max supported precision of {}", digits.len(), MAX_SUPPORTED_PRECISION - ))); + )}); } // Continue with the previous logic @@ -380,19 +380,17 @@ where (1, value.into()) }; if abs[1] != 0 || abs[2] != 0 || abs[3] != 0 { - return Err(ScalarConversionError::Overflow(format!( - "{} is too large to fit in an i8", - value - ))); + return Err(ScalarConversionError::Overflow { + error: format!("{} is too large to fit in an i8", value), + }); } let val: i128 = sign * abs[0] as i128; match val { 0 => Ok(false), 1 => Ok(true), - _ => Err(ScalarConversionError::Overflow(format!( - "{} is too large to fit in a bool", - value - ))), + _ => Err(ScalarConversionError::Overflow { + error: format!("{} is too large to fit in a bool", value), + }), } } } @@ -410,14 +408,13 @@ where (1, value.into()) }; if abs[1] != 0 || abs[2] != 0 || abs[3] != 0 { - return Err(ScalarConversionError::Overflow(format!( - "{} is too large to fit in an i8", - value - ))); + return Err(ScalarConversionError::Overflow { + error: format!("{} is too large to fit in an i8", value), + }); } let val: i128 = sign * abs[0] as i128; - val.try_into().map_err(|_| { - ScalarConversionError::Overflow(format!("{} is too large to fit in an i8", value)) + val.try_into().map_err(|_| ScalarConversionError::Overflow { + error: format!("{} is too large to fit in an i8", value), }) } } @@ -435,14 +432,13 @@ where (1, value.into()) }; if abs[1] != 0 || abs[2] != 0 || abs[3] != 0 { - return Err(ScalarConversionError::Overflow(format!( - "{} is too large to fit in an i16", - value - ))); + return Err(ScalarConversionError::Overflow { + error: format!("{} is too large to fit in an i16", value), + }); } let val: i128 = sign * abs[0] as i128; - val.try_into().map_err(|_| { - ScalarConversionError::Overflow(format!("{} is too large to fit in an i16", value)) + val.try_into().map_err(|_| ScalarConversionError::Overflow { + error: format!("{} is too large to fit in an i16", value), }) } } @@ -460,14 +456,13 @@ where (1, value.into()) }; if abs[1] != 0 || abs[2] != 0 || abs[3] != 0 { - return Err(ScalarConversionError::Overflow(format!( - "{} is too large to fit in an i32", - value - ))); + return Err(ScalarConversionError::Overflow { + error: format!("{} is too large to fit in an i32", value), + }); } let val: i128 = sign * abs[0] as i128; - val.try_into().map_err(|_| { - ScalarConversionError::Overflow(format!("{} is too large to fit in an i32", value)) + val.try_into().map_err(|_| ScalarConversionError::Overflow { + error: format!("{} is too large to fit in an i32", value), }) } } @@ -485,14 +480,13 @@ where (1, value.into()) }; if abs[1] != 0 || abs[2] != 0 || abs[3] != 0 { - return Err(ScalarConversionError::Overflow(format!( - "{} is too large to fit in an i64", - value - ))); + return Err(ScalarConversionError::Overflow { + error: format!("{} is too large to fit in an i64", value), + }); } let val: i128 = sign * abs[0] as i128; - val.try_into().map_err(|_| { - ScalarConversionError::Overflow(format!("{} is too large to fit in an i64", value)) + val.try_into().map_err(|_| ScalarConversionError::Overflow { + error: format!("{} is too large to fit in an i64", value), }) } } @@ -510,20 +504,18 @@ where (1, value.into()) }; if abs[2] != 0 || abs[3] != 0 { - return Err(ScalarConversionError::Overflow(format!( - "{} is too large to fit in an i128", - value - ))); + return Err(ScalarConversionError::Overflow { + error: format!("{} is too large to fit in an i128", value), + }); } let val: u128 = (abs[1] as u128) << 64 | (abs[0] as u128); match (sign, val) { (1, v) if v <= i128::MAX as u128 => Ok(v as i128), (-1, v) if v <= i128::MAX as u128 => Ok(-(v as i128)), (-1, v) if v == i128::MAX as u128 + 1 => Ok(i128::MIN), - _ => Err(ScalarConversionError::Overflow(format!( - "{} is too large to fit in an i128", - value - ))), + _ => Err(ScalarConversionError::Overflow { + error: format!("{} is too large to fit in an i128", value), + }), } } } diff --git a/crates/proof-of-sql/src/base/scalar/mont_scalar_test.rs b/crates/proof-of-sql/src/base/scalar/mont_scalar_test.rs index 96a033416..d1502ee7c 100644 --- a/crates/proof-of-sql/src/base/scalar/mont_scalar_test.rs +++ b/crates/proof-of-sql/src/base/scalar/mont_scalar_test.rs @@ -114,15 +114,15 @@ fn test_curve25519_scalar_to_bool() { fn test_curve25519_scalar_to_bool_overflow() { matches!( bool::try_from(Curve25519Scalar::from(2)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( bool::try_from(Curve25519Scalar::from(-1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( bool::try_from(Curve25519Scalar::from(-2)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); } @@ -145,11 +145,11 @@ fn test_curve25519_scalar_to_i8() { fn test_curve25519_scalar_to_i8_overflow() { matches!( i8::try_from(Curve25519Scalar::from(i8::MAX as i128 + 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( i8::try_from(Curve25519Scalar::from(i8::MIN as i128 - 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); } @@ -172,11 +172,11 @@ fn test_curve25519_scalar_to_i16() { fn test_curve25519_scalar_to_i16_overflow() { matches!( i16::try_from(Curve25519Scalar::from(i16::MAX as i128 + 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( i16::try_from(Curve25519Scalar::from(i16::MIN as i128 - 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); } @@ -199,11 +199,11 @@ fn test_curve25519_scalar_to_i32() { fn test_curve25519_scalar_to_i32_overflow() { matches!( i32::try_from(Curve25519Scalar::from(i32::MAX as i128 + 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( i32::try_from(Curve25519Scalar::from(i32::MIN as i128 - 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); } @@ -226,11 +226,11 @@ fn test_curve25519_scalar_to_i64() { fn test_curve25519_scalar_to_i64_overflow() { matches!( i64::try_from(Curve25519Scalar::from(i64::MAX as i128 + 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( i64::try_from(Curve25519Scalar::from(i64::MIN as i128 - 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); } @@ -253,11 +253,11 @@ fn test_curve25519_scalar_to_i128() { fn test_curve25519_scalar_to_i128_overflow() { matches!( i128::try_from(Curve25519Scalar::from(i128::MAX) + Curve25519Scalar::ONE), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( i128::try_from(Curve25519Scalar::from(i128::MIN) - Curve25519Scalar::ONE), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); } diff --git a/crates/proof-of-sql/src/proof_primitive/dory/dory_commitment_evaluation_proof.rs b/crates/proof-of-sql/src/proof_primitive/dory/dory_commitment_evaluation_proof.rs index f344f6eaa..63867b948 100644 --- a/crates/proof-of-sql/src/proof_primitive/dory/dory_commitment_evaluation_proof.rs +++ b/crates/proof-of-sql/src/proof_primitive/dory/dory_commitment_evaluation_proof.rs @@ -7,23 +7,23 @@ use super::{ }; use crate::base::commitment::CommitmentEvaluationProof; use merlin::Transcript; -use thiserror::Error; +use snafu::Snafu; /// The `CommitmentEvaluationProof` for the Dory PCS. pub type DoryEvaluationProof = DoryMessages; /// The error type for the Dory PCS. -#[derive(Error, Debug)] +#[derive(Snafu, Debug)] pub enum DoryError { /// This error occurs when the generators offset is invalid. - #[error("invalid generators offset: {0}")] - InvalidGeneratorsOffset(u64), + #[snafu(display("invalid generators offset: {offset}"))] + InvalidGeneratorsOffset { offset: u64 }, /// This error occurs when the proof fails to verify. - #[error("verification error")] + #[snafu(display("verification error"))] VerificationError, /// This error occurs when the setup is too small. - #[error("setup is too small: the setup is {0}, but the proof requires a setup of size {1}")] - SmallSetup(usize, usize), + #[snafu(display("setup is too small: the setup is {actual}, but the proof requires a setup of size {required}"))] + SmallSetup { actual: usize, required: usize }, } impl CommitmentEvaluationProof for DoryEvaluationProof { @@ -85,14 +85,19 @@ impl CommitmentEvaluationProof for DoryEvaluationProof { ); // Dory PCS Logic if generators_offset != 0 { - return Err(DoryError::InvalidGeneratorsOffset(generators_offset)); + return Err(DoryError::InvalidGeneratorsOffset { + offset: generators_offset, + }); } let b_point: &[F] = bytemuck::TransparentWrapper::peel_slice(b_point); let verifier_setup = setup.verifier_setup(); let mut messages = self.clone(); let nu = compute_nu(b_point.len(), setup.sigma()); if nu > verifier_setup.max_nu { - return Err(DoryError::SmallSetup(verifier_setup.max_nu, nu)); + return Err(DoryError::SmallSetup { + actual: verifier_setup.max_nu, + required: nu, + }); } let state = build_vmv_verifier_state(product.0, b_point, a_commit, setup.sigma(), nu); let extended_state = eval_vmv_re_verify(&mut messages, transcript, state, verifier_setup) diff --git a/crates/proof-of-sql/src/proof_primitive/dory/dory_commitment_test.rs b/crates/proof-of-sql/src/proof_primitive/dory/dory_commitment_test.rs index eb536cfae..670d649cf 100644 --- a/crates/proof-of-sql/src/proof_primitive/dory/dory_commitment_test.rs +++ b/crates/proof-of-sql/src/proof_primitive/dory/dory_commitment_test.rs @@ -13,15 +13,15 @@ fn test_dory_scalar_to_bool() { fn test_dory_scalar_to_bool_overflow() { matches!( bool::try_from(DoryScalar::from(2)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( bool::try_from(DoryScalar::from(-1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( bool::try_from(DoryScalar::from(-2)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); } @@ -38,11 +38,11 @@ fn test_dory_scalar_to_i8() { fn test_dory_scalar_to_i8_overflow() { matches!( i8::try_from(DoryScalar::from(i8::MAX as i128 + 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( i8::try_from(DoryScalar::from(i8::MIN as i128 - 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); } @@ -59,11 +59,11 @@ fn test_dory_scalar_to_i16() { fn test_dory_scalar_to_i16_overflow() { matches!( i16::try_from(DoryScalar::from(i16::MAX as i128 + 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( i16::try_from(DoryScalar::from(i16::MIN as i128 - 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); } @@ -80,11 +80,11 @@ fn test_dory_scalar_to_i32() { fn test_dory_scalar_to_i32_overflow() { matches!( i32::try_from(DoryScalar::from(i32::MAX as i128 + 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( i32::try_from(DoryScalar::from(i32::MIN as i128 - 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); } @@ -101,11 +101,11 @@ fn test_dory_scalar_to_i64() { fn test_dory_scalar_to_i64_overflow() { matches!( i64::try_from(DoryScalar::from(i64::MAX as i128 + 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( i64::try_from(DoryScalar::from(i64::MIN as i128 - 1)), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); } @@ -128,11 +128,11 @@ fn test_dory_scalar_to_i128() { fn test_dory_scalar_to_i128_overflow() { matches!( i128::try_from(DoryScalar::from(i128::MAX) + DoryScalar::ONE), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); matches!( i128::try_from(DoryScalar::from(i128::MIN) - DoryScalar::ONE), - Err(ScalarConversionError::Overflow(_)) + Err(ScalarConversionError::Overflow { .. }) ); } diff --git a/crates/proof-of-sql/src/proof_primitive/dory/dynamic_dory_commitment_evaluation_proof.rs b/crates/proof-of-sql/src/proof_primitive/dory/dynamic_dory_commitment_evaluation_proof.rs index d8ce51eb5..3347a04ba 100644 --- a/crates/proof-of-sql/src/proof_primitive/dory/dynamic_dory_commitment_evaluation_proof.rs +++ b/crates/proof-of-sql/src/proof_primitive/dory/dynamic_dory_commitment_evaluation_proof.rs @@ -8,24 +8,24 @@ use super::{ use crate::base::commitment::CommitmentEvaluationProof; use merlin::Transcript; use serde::{Deserialize, Serialize}; -use thiserror::Error; +use snafu::Snafu; /// The `CommitmentEvaluationProof` for the Dory PCS. #[derive(Default, Clone, Serialize, Deserialize, PartialEq, Eq, Debug)] pub struct DynamicDoryEvaluationProof(pub(super) DoryMessages); /// The error type for the Dory PCS. -#[derive(Error, Debug)] +#[derive(Snafu, Debug)] pub enum DoryError { /// This error occurs when the generators offset is invalid. - #[error("invalid generators offset: {0}")] - InvalidGeneratorsOffset(u64), + #[snafu(display("invalid generators offset: {offset}"))] + InvalidGeneratorsOffset { offset: u64 }, /// This error occurs when the proof fails to verify. - #[error("verification error")] + #[snafu(display("verification error"))] VerificationError, /// This error occurs when the setup is too small. - #[error("setup is too small: the setup is {0}, but the proof requires a setup of size {1}")] - SmallSetup(usize, usize), + #[snafu(display("setup is too small: the setup is {actual}, but the proof requires a setup of size {required}"))] + SmallSetup { actual: usize, required: usize }, } impl CommitmentEvaluationProof for DynamicDoryEvaluationProof { @@ -86,13 +86,18 @@ impl CommitmentEvaluationProof for DynamicDoryEvaluationProof { ); // Dory PCS Logic if generators_offset != 0 { - return Err(DoryError::InvalidGeneratorsOffset(generators_offset)); + return Err(DoryError::InvalidGeneratorsOffset { + offset: generators_offset, + }); } let b_point: &[F] = bytemuck::TransparentWrapper::peel_slice(b_point); let mut messages = self.0.clone(); let nu = compute_dynamic_nu(b_point.len()); if nu > setup.max_nu { - return Err(DoryError::SmallSetup(setup.max_nu, nu)); + return Err(DoryError::SmallSetup { + actual: setup.max_nu, + required: nu, + }); } let state = build_dynamic_vmv_verifier_state(product.0, b_point, a_commit, nu); let extended_state = eval_vmv_re_verify(&mut messages, transcript, state, setup) diff --git a/crates/proof-of-sql/src/proof_primitive/sumcheck/proof.rs b/crates/proof-of-sql/src/proof_primitive/sumcheck/proof.rs index 48315558b..3f1693851 100644 --- a/crates/proof-of-sql/src/proof_primitive/sumcheck/proof.rs +++ b/crates/proof-of-sql/src/proof_primitive/sumcheck/proof.rs @@ -65,9 +65,9 @@ impl SumcheckProof { // This challenge is in order to keep transcript messages grouped. (This simplifies the Solidity implementation.) transcript.scalar_challenge_as_be::(); if self.evaluations.len() != polynomial_info.num_variables { - return Err(ProofError::VerificationError( - "invalid number of evaluations", - )); + return Err(ProofError::VerificationError { + error: "invalid number of evaluations", + }); } let mut evaluation_point = Vec::with_capacity(polynomial_info.num_variables); for round_index in 0..polynomial_info.num_variables { diff --git a/crates/proof-of-sql/src/proof_primitive/sumcheck/subclaim.rs b/crates/proof-of-sql/src/proof_primitive/sumcheck/subclaim.rs index f0ec1f422..deaf632fe 100644 --- a/crates/proof-of-sql/src/proof_primitive/sumcheck/subclaim.rs +++ b/crates/proof-of-sql/src/proof_primitive/sumcheck/subclaim.rs @@ -25,14 +25,14 @@ impl Subclaim { for round_index in 0..num_vars { let round_evaluation = &evaluations[round_index]; if round_evaluation.len() != max_multiplicands + 1 { - return Err(ProofError::VerificationError( - "round evaluation length does not match max multiplicands", - )); + return Err(ProofError::VerificationError { + error: "round evaluation length does not match max multiplicands", + }); } if expected_sum != round_evaluation[0] + round_evaluation[1] { - return Err(ProofError::VerificationError( - "round evaluation does not match claimed sum", - )); + return Err(ProofError::VerificationError { + error: "round evaluation does not match claimed sum", + }); } expected_sum = interpolate_uni_poly(round_evaluation, evaluation_point[round_index]); } diff --git a/crates/proof-of-sql/src/sql/parse/dyn_proof_expr_builder.rs b/crates/proof-of-sql/src/sql/parse/dyn_proof_expr_builder.rs index a3ca4603f..39517a44e 100644 --- a/crates/proof-of-sql/src/sql/parse/dyn_proof_expr_builder.rs +++ b/crates/proof-of-sql/src/sql/parse/dyn_proof_expr_builder.rs @@ -60,10 +60,9 @@ impl DynProofExprBuilder<'_> { Expression::Binary { op, left, right } => self.visit_binary_expr(*op, left, right), Expression::Unary { op, expr } => self.visit_unary_expr(*op, expr), Expression::Aggregation { op, expr } => self.visit_aggregate_expr(*op, expr), - _ => Err(ConversionError::Unprovable(format!( - "Expression {:?} is not supported yet", - expr - ))), + _ => Err(ConversionError::Unprovable { + error: format!("Expression {:?} is not supported yet", expr), + }), } } @@ -73,7 +72,9 @@ impl DynProofExprBuilder<'_> { ) -> Result, ConversionError> { Ok(DynProofExpr::Column(ColumnExpr::new( *self.column_mapping.get(&identifier).ok_or( - ConversionError::MissingColumnWithoutTable(Box::new(identifier)), + ConversionError::MissingColumnWithoutTable { + identifier: Box::new(identifier), + }, )?, ))) } @@ -88,9 +89,12 @@ impl DynProofExprBuilder<'_> { Literal::Int128(i) => Ok(DynProofExpr::new_literal(LiteralValue::Int128(*i))), Literal::Decimal(d) => { let scale = d.scale(); - let precision = Precision::new(d.precision()).map_err(|_| { - DecimalConversionError(InvalidPrecision(d.precision().to_string())) - })?; + let precision = + Precision::new(d.precision()).map_err(|_| DecimalConversionError { + source: InvalidPrecision { + error: d.precision().to_string(), + }, + })?; Ok(DynProofExpr::new_literal(LiteralValue::Decimal75( precision, scale, @@ -105,10 +109,10 @@ impl DynProofExprBuilder<'_> { let timestamp = match its.timeunit() { PoSQLTimeUnit::Nanosecond => { its.timestamp().timestamp_nanos_opt().ok_or_else(|| { - PoSQLTimestampError::UnsupportedPrecision("Timestamp out of range: + PoSQLTimestampError::UnsupportedPrecision{ error: "Timestamp out of range: Valid nanosecond timestamps must be between 1677-09-21T00:12:43.145224192 and 2262-04-11T23:47:16.854775807.".to_owned() - ) + } })? } PoSQLTimeUnit::Microsecond => its.timestamp().timestamp_micros(), @@ -183,10 +187,9 @@ impl DynProofExprBuilder<'_> { let right = self.visit_expr(right); DynProofExpr::try_new_multiply(left?, right?) } - BinaryOperator::Division => Err(ConversionError::Unprovable(format!( - "Binary operator {:?} is not supported at this location", - op - ))), + BinaryOperator::Division => Err(ConversionError::Unprovable { + error: format!("Binary operator {:?} is not supported at this location", op), + }), } } @@ -196,23 +199,27 @@ impl DynProofExprBuilder<'_> { expr: &Expression, ) -> Result, ConversionError> { if self.in_agg_scope { - return Err(ConversionError::InvalidExpression( - "nested aggregations are invalid".to_string(), - )); + return Err(ConversionError::InvalidExpression { + expression: "nested aggregations are invalid".to_string(), + }); } let expr = DynProofExprBuilder::new_agg(self.column_mapping).visit_expr(expr)?; match (op, expr.data_type().is_numeric()) { (AggregationOperator::Count, _) | (AggregationOperator::Sum, true) => { Ok(DynProofExpr::new_aggregate(op, expr)) } - (AggregationOperator::Sum, false) => Err(ConversionError::InvalidExpression(format!( - "Aggregation operator {:?} doesn't work with non-numeric types", - op - ))), - _ => Err(ConversionError::Unprovable(format!( - "Aggregation operator {:?} is not supported at this location", - op - ))), + (AggregationOperator::Sum, false) => Err(ConversionError::InvalidExpression { + expression: format!( + "Aggregation operator {:?} doesn't work with non-numeric types", + op + ), + }), + _ => Err(ConversionError::Unprovable { + error: format!( + "Aggregation operator {:?} is not supported at this location", + op + ), + }), } } } diff --git a/crates/proof-of-sql/src/sql/parse/error.rs b/crates/proof-of-sql/src/sql/parse/error.rs index 745db218b..b6a1d72e3 100644 --- a/crates/proof-of-sql/src/sql/parse/error.rs +++ b/crates/proof-of-sql/src/sql/parse/error.rs @@ -6,20 +6,28 @@ use proof_of_sql_parser::{ intermediate_decimal::IntermediateDecimalError, posql_time::PoSQLTimestampError, Identifier, ResourceId, }; -use thiserror::Error; +use snafu::Snafu; /// Errors from converting an intermediate AST into a provable AST. -#[derive(Error, Debug, PartialEq, Eq)] +#[derive(Snafu, Debug, PartialEq, Eq)] pub enum ConversionError { - #[error("Column '{0}' was not found in table '{1}'")] + #[snafu(display("Column '{identifier}' was not found in table '{resource_id}'"))] /// The column is missing in the table - MissingColumn(Box, Box), + MissingColumn { + /// The missing column identifier + identifier: Box, + /// The table resource id + resource_id: Box, + }, - #[error("Column '{0}' was not found")] + #[snafu(display("Column '{identifier}' was not found"))] /// The column is missing (without table information) - MissingColumnWithoutTable(Box), + MissingColumnWithoutTable { + /// The missing column identifier + identifier: Box, + }, - #[error("Expected '{expected}' but found '{actual}'")] + #[snafu(display("Expected '{expected}' but found '{actual}'"))] /// Invalid data type received InvalidDataType { /// Expected data type @@ -28,62 +36,111 @@ pub enum ConversionError { actual: ColumnType, }, - #[error("Left side has '{1}' type but right side has '{0}' type")] + #[snafu(display("Left side has '{left_type}' type but right side has '{right_type}' type"))] /// Data types do not match - DataTypeMismatch(String, String), + DataTypeMismatch { + /// The left side datatype + left_type: String, + /// The right side datatype + right_type: String, + }, - #[error("Columns have different lengths: {0} != {1}")] + #[snafu(display("Columns have different lengths: {len_a} != {len_b}"))] /// Two columns do not have the same length - DifferentColumnLength(usize, usize), + DifferentColumnLength { + /// The length of the first column + len_a: usize, + /// The length of the second column + len_b: usize, + }, - #[error("Multiple result columns with the same alias '{0}' have been found.")] + #[snafu(display("Multiple result columns with the same alias '{alias}' have been found."))] /// Duplicate alias in result columns - DuplicateResultAlias(String), + DuplicateResultAlias { + /// The duplicate alias + alias: String, + }, - #[error("A WHERE clause must has boolean type. It is currently of type '{0}'.")] + #[snafu(display( + "A WHERE clause must has boolean type. It is currently of type '{datatype}'." + ))] /// WHERE clause is not boolean - NonbooleanWhereClause(ColumnType), + NonbooleanWhereClause { + /// The actual datatype of the WHERE clause + datatype: ColumnType, + }, - #[error("Invalid order by: alias '{0}' does not appear in the result expressions.")] + #[snafu(display( + "Invalid order by: alias '{alias}' does not appear in the result expressions." + ))] /// ORDER BY clause references a non-existent alias - InvalidOrderBy(String), + InvalidOrderBy { + /// The non-existent alias in the ORDER BY clause + alias: String, + }, - #[error("Invalid group by: column '{0}' must appear in the group by expression.")] + #[snafu(display( + "Invalid group by: column '{column}' must appear in the group by expression." + ))] /// GROUP BY clause references a non-existent column - InvalidGroupByColumnRef(String), + InvalidGroupByColumnRef { + /// The non-existent column in the GROUP BY clause + column: String, + }, - #[error("Invalid expression: {0}")] + #[snafu(display("Invalid expression: {expression}"))] /// General error for invalid expressions - InvalidExpression(String), + InvalidExpression { + /// The invalid expression error + expression: String, + }, - #[error("Encountered parsing error: {0}")] + #[snafu(display("Encountered parsing error: {error}"))] /// General parsing error - ParseError(String), + ParseError { + /// The underlying error + error: String, + }, - #[error(transparent)] + #[snafu(transparent)] /// Errors related to decimal operations - DecimalConversionError(#[from] DecimalError), + DecimalConversionError { + /// The underlying source error + source: DecimalError, + }, /// Errors related to timestamp parsing - #[error("Timestamp conversion error: {0}")] - TimestampConversionError(#[from] PoSQLTimestampError), + #[snafu(context(false), display("Timestamp conversion error: {source}"))] + TimestampConversionError { + /// The underlying source error + source: PoSQLTimestampError, + }, /// Errors related to column operations - #[error(transparent)] - ColumnOperationError(#[from] ColumnOperationError), + #[snafu(transparent)] + ColumnOperationError { + /// The underlying source error + source: ColumnOperationError, + }, /// Errors related to postprocessing - #[error(transparent)] - PostprocessingError(#[from] crate::sql::postprocessing::PostprocessingError), + #[snafu(transparent)] + PostprocessingError { + /// The underlying source error + source: crate::sql::postprocessing::PostprocessingError, + }, - #[error("Query not provable because: {0}")] + #[snafu(display("Query not provable because: {error}"))] /// Query requires unprovable feature - Unprovable(String), + Unprovable { + /// The underlying error + error: String, + }, } impl From for ConversionError { fn from(value: String) -> Self { - ConversionError::ParseError(value) + ConversionError::ParseError { error: value } } } @@ -95,20 +152,22 @@ impl From for String { impl From for ConversionError { fn from(err: IntermediateDecimalError) -> ConversionError { - ConversionError::DecimalConversionError(DecimalError::IntermediateDecimalConversionError( - err, - )) + ConversionError::DecimalConversionError { + source: DecimalError::IntermediateDecimalConversionError { source: err }, + } } } impl ConversionError { /// Returns a `ConversionError::InvalidExpression` for non-numeric types used in numeric aggregation functions. pub fn non_numeric_expr_in_agg>(dtype: S, func: S) -> Self { - ConversionError::InvalidExpression(format!( - "cannot use expression of type '{}' with numeric aggregation function '{}'", - dtype.into().to_lowercase(), - func.into().to_lowercase() - )) + ConversionError::InvalidExpression { + expression: format!( + "cannot use expression of type '{}' with numeric aggregation function '{}'", + dtype.into().to_lowercase(), + func.into().to_lowercase() + ), + } } } diff --git a/crates/proof-of-sql/src/sql/parse/query_context.rs b/crates/proof-of-sql/src/sql/parse/query_context.rs index 941d36dbd..e43982b70 100644 --- a/crates/proof-of-sql/src/sql/parse/query_context.rs +++ b/crates/proof-of-sql/src/sql/parse/query_context.rs @@ -77,9 +77,9 @@ impl QueryContext { if self.in_agg_scope { // TODO: Disable this once we support nested aggregations - return Err(ConversionError::InvalidExpression( - "nested aggregations are not supported".to_string(), - )); + return Err(ConversionError::InvalidExpression { + expression: "nested aggregations are not supported".to_string(), + }); } self.agg_counter += 1; @@ -147,7 +147,9 @@ impl QueryContext { .iter() .find(|group_column| *group_column == column) .map(|_| true) - .ok_or(ConversionError::InvalidGroupByColumnRef(column.to_string())) + .ok_or(ConversionError::InvalidGroupByColumnRef { + column: column.to_string(), + }) } pub fn get_aliased_result_exprs(&self) -> ConversionResult<&[AliasedResultExpr]> { @@ -162,7 +164,9 @@ impl QueryContext { .sum::() != 1 { - return Err(ConversionError::DuplicateResultAlias(col.alias.to_string())); + return Err(ConversionError::DuplicateResultAlias { + alias: col.alias.to_string(), + }); } } @@ -171,9 +175,9 @@ impl QueryContext { && self.agg_counter > 0 && self.first_result_col_out_agg_scope.is_some() { - return Err(ConversionError::InvalidGroupByColumnRef( - self.first_result_col_out_agg_scope.unwrap().to_string(), - )); + return Err(ConversionError::InvalidGroupByColumnRef { + column: self.first_result_col_out_agg_scope.unwrap().to_string(), + }); } Ok(&self.res_aliased_exprs) @@ -185,9 +189,9 @@ impl QueryContext { self.res_aliased_exprs .iter() .find(|col| col.alias == by_expr.expr) - .ok_or(ConversionError::InvalidOrderBy( - by_expr.expr.as_str().to_string(), - ))?; + .ok_or(ConversionError::InvalidOrderBy { + alias: by_expr.expr.as_str().to_string(), + })?; } Ok(self.order_by_exprs.clone()) @@ -222,7 +226,9 @@ impl TryFrom<&QueryContext> for Option> { .build(value.where_expr.clone())? .unwrap_or_else(|| DynProofExpr::new_literal(LiteralValue::Boolean(true))); let table = value.table.map(|table_ref| TableExpr { table_ref }).ok_or( - ConversionError::InvalidExpression("QueryContext has no table_ref".to_owned()), + ConversionError::InvalidExpression { + expression: "QueryContext has no table_ref".to_owned(), + }, )?; let resource_id = table.table_ref.resource_id(); let group_by_exprs = value @@ -232,10 +238,10 @@ impl TryFrom<&QueryContext> for Option> { value .column_mapping .get(expr) - .ok_or(ConversionError::MissingColumn( - Box::new(*expr), - Box::new(resource_id), - )) + .ok_or(ConversionError::MissingColumn { + identifier: Box::new(*expr), + resource_id: Box::new(resource_id), + }) .map(|column_ref| ColumnExpr::::new(*column_ref)) }) .collect::>, ConversionError>>()?; diff --git a/crates/proof-of-sql/src/sql/parse/query_context_builder.rs b/crates/proof-of-sql/src/sql/parse/query_context_builder.rs index 7ede823db..1c1b0aacc 100644 --- a/crates/proof-of-sql/src/sql/parse/query_context_builder.rs +++ b/crates/proof-of-sql/src/sql/parse/query_context_builder.rs @@ -229,8 +229,9 @@ impl<'a> QueryContextBuilder<'a> { let table_ref = self.context.get_table_ref(); let column_type = self.schema_accessor.lookup_column(*table_ref, column_name); - let column_type = column_type.ok_or_else(|| { - ConversionError::MissingColumn(Box::new(column_name), Box::new(table_ref.resource_id())) + let column_type = column_type.ok_or_else(|| ConversionError::MissingColumn { + identifier: Box::new(column_name), + resource_id: Box::new(table_ref.resource_id()), })?; let column = ColumnRef::new(*table_ref, column_name, column_type); @@ -306,9 +307,9 @@ fn check_dtypes( if type_check_binary_operation(&left_dtype, &right_dtype, binary_operator) { Ok(()) } else { - Err(ConversionError::DataTypeMismatch( - left_dtype.to_string(), - right_dtype.to_string(), - )) + Err(ConversionError::DataTypeMismatch { + left_type: left_dtype.to_string(), + right_type: right_dtype.to_string(), + }) } } diff --git a/crates/proof-of-sql/src/sql/parse/query_expr_tests.rs b/crates/proof-of-sql/src/sql/parse/query_expr_tests.rs index d5be5b0ad..85bc340b6 100644 --- a/crates/proof-of-sql/src/sql/parse/query_expr_tests.rs +++ b/crates/proof-of-sql/src/sql/parse/query_expr_tests.rs @@ -1591,39 +1591,39 @@ fn we_can_parse_arithmetic_expression_within_aggregations_in_the_result_expr() { fn we_cannot_use_non_grouped_columns_outside_agg() { assert!(matches!( query!(select: ["i"], group: ["s"], should_err: true), - ConversionError::PostprocessingError( - PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause(_) - ) + ConversionError::PostprocessingError { + source: PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause { .. } + } )); assert!(matches!( query!(select: ["sum(i)", "i"], group: ["s"], should_err: true), - ConversionError::PostprocessingError( - PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause(_) - ) + ConversionError::PostprocessingError { + source: PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause { .. } + } )); assert!(matches!( query!(select: ["min(i) + i"], group: ["s"], should_err: true), - ConversionError::PostprocessingError( - PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause(_) - ) + ConversionError::PostprocessingError { + source: PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause { .. } + } )); assert!(matches!( query!(select: ["2 * i", "min(i)"], group: ["s"], should_err: true), - ConversionError::PostprocessingError( - PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause(_) - ) + ConversionError::PostprocessingError { + source: PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause { .. } + } )); assert!(matches!( query!(select: ["2 * i", "min(i)"], should_err: true), - ConversionError::InvalidGroupByColumnRef(_) + ConversionError::InvalidGroupByColumnRef { .. } )); assert!(matches!( query!(select: ["sum(i)", "i"], should_err: true), - ConversionError::InvalidGroupByColumnRef(_) + ConversionError::InvalidGroupByColumnRef { .. } )); assert!(matches!( query!(select: ["max(i) + 2 * i"], should_err: true), - ConversionError::InvalidGroupByColumnRef(_) + ConversionError::InvalidGroupByColumnRef { .. } )); } @@ -1631,31 +1631,31 @@ fn we_cannot_use_non_grouped_columns_outside_agg() { fn varchar_column_is_not_compatible_with_integer_column() { assert_eq!( query!(select: ["-123 * s"], should_err: true), - ConversionError::DataTypeMismatch( - ColumnType::BigInt.to_string(), - ColumnType::VarChar.to_string() - ) + ConversionError::DataTypeMismatch { + left_type: ColumnType::BigInt.to_string(), + right_type: ColumnType::VarChar.to_string() + } ); assert_eq!( query!(select: ["i - s"], should_err: true), - ConversionError::DataTypeMismatch( - ColumnType::BigInt.to_string(), - ColumnType::VarChar.to_string() - ) + ConversionError::DataTypeMismatch { + left_type: ColumnType::BigInt.to_string(), + right_type: ColumnType::VarChar.to_string() + } ); assert_eq!( query!(select: ["s"], filter: "'abc' = i", should_err: true), - ConversionError::DataTypeMismatch( - ColumnType::VarChar.to_string(), - ColumnType::BigInt.to_string(), - ) + ConversionError::DataTypeMismatch { + left_type: ColumnType::VarChar.to_string(), + right_type: ColumnType::BigInt.to_string(), + } ); assert_eq!( query!(select: ["s"], filter: "'abc' != i", should_err: true), - ConversionError::DataTypeMismatch( - ColumnType::VarChar.to_string(), - ColumnType::BigInt.to_string(), - ) + ConversionError::DataTypeMismatch { + left_type: ColumnType::VarChar.to_string(), + right_type: ColumnType::BigInt.to_string(), + } ); } @@ -1663,10 +1663,10 @@ fn varchar_column_is_not_compatible_with_integer_column() { fn arithmetic_operations_are_not_allowed_with_varchar_column() { assert_eq!( query!(select: ["s - s1"], should_err: true), - ConversionError::DataTypeMismatch( - ColumnType::VarChar.to_string(), - ColumnType::VarChar.to_string() - ) + ConversionError::DataTypeMismatch { + left_type: ColumnType::VarChar.to_string(), + right_type: ColumnType::VarChar.to_string() + } ); } @@ -1866,7 +1866,9 @@ fn nested_aggregations_are_not_supported() { for perm_aggs in supported_agg.iter().permutations(2) { assert_eq!( query!(select: [format!("{}({}(i))", perm_aggs[0], perm_aggs[1])], should_err: true), - ConversionError::InvalidExpression("nested aggregations are not supported".to_string()) + ConversionError::InvalidExpression { + expression: "nested aggregations are not supported".to_string() + } ); } } diff --git a/crates/proof-of-sql/src/sql/parse/where_expr_builder.rs b/crates/proof-of-sql/src/sql/parse/where_expr_builder.rs index 5636518e5..e74761012 100644 --- a/crates/proof-of-sql/src/sql/parse/where_expr_builder.rs +++ b/crates/proof-of-sql/src/sql/parse/where_expr_builder.rs @@ -33,9 +33,9 @@ impl<'a> WhereExprBuilder<'a> { // Ensure that the expression is a boolean expression match expr_plan.data_type() { ColumnType::Boolean => Ok(expr_plan), - _ => Err(ConversionError::NonbooleanWhereClause( - expr_plan.data_type(), - )), + _ => Err(ConversionError::NonbooleanWhereClause { + datatype: expr_plan.data_type(), + }), } }) .transpose() diff --git a/crates/proof-of-sql/src/sql/parse/where_expr_builder_tests.rs b/crates/proof-of-sql/src/sql/parse/where_expr_builder_tests.rs index 34c906c88..ab11ed621 100644 --- a/crates/proof-of-sql/src/sql/parse/where_expr_builder_tests.rs +++ b/crates/proof-of-sql/src/sql/parse/where_expr_builder_tests.rs @@ -261,7 +261,7 @@ fn we_can_not_have_missing_column_as_where_clause() { let res = builder.build::(Some(expr_missing)); assert!(matches!( res, - Result::Err(ConversionError::MissingColumnWithoutTable(_)) + Result::Err(ConversionError::MissingColumnWithoutTable { .. }) )); } @@ -275,7 +275,7 @@ fn we_can_not_have_non_boolean_column_as_where_clause() { let res = builder.build::(Some(expr_non_boolean)); assert!(matches!( res, - Result::Err(ConversionError::NonbooleanWhereClause(_)) + Result::Err(ConversionError::NonbooleanWhereClause { .. }) )); } @@ -289,7 +289,7 @@ fn we_can_not_have_non_boolean_literal_as_where_clause() { let res = builder.build::(Some(expr_non_boolean)); assert!(matches!( res, - Result::Err(ConversionError::NonbooleanWhereClause(_)) + Result::Err(ConversionError::NonbooleanWhereClause { .. }) )); } @@ -308,7 +308,7 @@ fn we_expect_an_error_while_trying_to_check_varchar_column_eq_decimal() { t.schema_id(), &accessor, ), - Err(ConversionError::DataTypeMismatch(_, _)) + Err(ConversionError::DataTypeMismatch { .. }) )); } @@ -327,7 +327,7 @@ fn we_expect_an_error_while_trying_to_check_varchar_column_ge_decimal() { t.schema_id(), &accessor, ), - Err(ConversionError::DataTypeMismatch(_, _)) + Err(ConversionError::DataTypeMismatch { .. }) )); } diff --git a/crates/proof-of-sql/src/sql/postprocessing/error.rs b/crates/proof-of-sql/src/sql/postprocessing/error.rs index d880c8dff..6e416d2a7 100644 --- a/crates/proof-of-sql/src/sql/postprocessing/error.rs +++ b/crates/proof-of-sql/src/sql/postprocessing/error.rs @@ -1,33 +1,57 @@ use proof_of_sql_parser::Identifier; -use thiserror::Error; +use snafu::Snafu; /// Errors in postprocessing -#[derive(Error, Debug, PartialEq, Eq)] +#[derive(Snafu, Debug, PartialEq, Eq)] pub enum PostprocessingError { /// Error in slicing due to slice index beyond usize - #[error("Error in slicing due to slice index beyond usize {0}")] - InvalidSliceIndex(i128), + #[snafu(display("Error in slicing due to slice index beyond usize {index}"))] + InvalidSliceIndex { + /// The overflowing index value + index: i128, + }, /// Column not found - #[error("Column not found: {0}")] - ColumnNotFound(String), + #[snafu(display("Column not found: {column}"))] + ColumnNotFound { + /// The column which is not found + column: String, + }, /// Errors in evaluation of `Expression`s - #[error(transparent)] - ExpressionEvaluationError(#[from] crate::base::database::ExpressionEvaluationError), + #[snafu(transparent)] + ExpressionEvaluationError { + /// The underlying source error + source: crate::base::database::ExpressionEvaluationError, + }, /// Errors in constructing `OwnedTable` - #[error(transparent)] - OwnedTableError(#[from] crate::base::database::OwnedTableError), + #[snafu(transparent)] + OwnedTableError { + /// The underlying source error + source: crate::base::database::OwnedTableError, + }, /// GROUP BY clause references a column not in a group by expression outside aggregate functions - #[error("Invalid group by: column '{0}' must not appear outside aggregate functions or `GROUP BY` clause.")] - IdentifierNotInAggregationOperatorOrGroupByClause(Identifier), + #[snafu(display("Invalid group by: column '{column}' must not appear outside aggregate functions or `GROUP BY` clause."))] + IdentifierNotInAggregationOperatorOrGroupByClause { + /// The column identifier + column: Identifier, + }, /// Errors in aggregate columns - #[error(transparent)] - AggregateColumnsError(#[from] crate::base::database::group_by_util::AggregateColumnsError), + #[snafu(transparent)] + AggregateColumnsError { + /// The underlying source error + source: crate::base::database::group_by_util::AggregateColumnsError, + }, /// Errors in `OwnedColumn` - #[error(transparent)] - OwnedColumnError(#[from] crate::base::database::OwnedColumnError), + #[snafu(transparent)] + OwnedColumnError { + /// The underlying source error + source: crate::base::database::OwnedColumnError, + }, /// Nested aggregation in `GROUP BY` clause - #[error("Nested aggregation in `GROUP BY` clause: {0}")] - NestedAggregationInGroupByClause(String), + #[snafu(display("Nested aggregation in `GROUP BY` clause: {error}"))] + NestedAggregationInGroupByClause { + /// The nested aggregation error + error: String, + }, } /// Result type for postprocessing diff --git a/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing.rs b/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing.rs index 44635932f..d8e6bd0eb 100644 --- a/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing.rs +++ b/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing.rs @@ -115,9 +115,9 @@ fn check_and_get_aggregation_and_remainder( .cloned() .collect::>(); if contains_nested_aggregation(&expr.expr, false) { - return Err(PostprocessingError::NestedAggregationInGroupByClause( - format!("Nested aggregations found {:?}", expr.expr), - )); + return Err(PostprocessingError::NestedAggregationInGroupByClause { + error: format!("Nested aggregations found {:?}", expr.expr), + }); } if free_identifiers.is_subset(&group_by_identifier_set) { let remainder = get_aggregate_and_remainder_expressions(*expr.expr, aggregation_expr_map); @@ -130,7 +130,11 @@ fn check_and_get_aggregation_and_remainder( .difference(&group_by_identifier_set) .next() .unwrap(); - Err(PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause(*diff)) + Err( + PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause { + column: *diff, + }, + ) } } @@ -198,10 +202,11 @@ impl PostprocessingStep for GroupByPostprocessing { .group_by_identifiers .iter() .map(|id| { - let column = owned_table - .inner_table() - .get(id) - .ok_or(PostprocessingError::ColumnNotFound(id.to_string()))?; + let column = owned_table.inner_table().get(id).ok_or( + PostprocessingError::ColumnNotFound { + column: id.to_string(), + }, + )?; Ok(Column::::from_owned_column(column, &alloc)) }) .collect::>>()?; diff --git a/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing_test.rs b/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing_test.rs index 44164144e..f0fc572ad 100644 --- a/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing_test.rs +++ b/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing_test.rs @@ -19,7 +19,7 @@ fn we_cannot_have_invalid_group_bys() { let res = GroupByPostprocessing::try_new(vec![ident("a")], vec![aliased_expr(expr, "res")]); assert!(matches!( res, - Err(PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause(_)) + Err(PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause { .. }) )); // Nested aggregation @@ -27,7 +27,7 @@ fn we_cannot_have_invalid_group_bys() { let res = GroupByPostprocessing::try_new(vec![ident("a")], vec![aliased_expr(expr, "res")]); assert!(matches!( res, - Err(PostprocessingError::NestedAggregationInGroupByClause(_)) + Err(PostprocessingError::NestedAggregationInGroupByClause { .. }) )); } diff --git a/crates/proof-of-sql/src/sql/postprocessing/order_by_postprocessing.rs b/crates/proof-of-sql/src/sql/postprocessing/order_by_postprocessing.rs index f8fe2f92e..9795b33b3 100644 --- a/crates/proof-of-sql/src/sql/postprocessing/order_by_postprocessing.rs +++ b/crates/proof-of-sql/src/sql/postprocessing/order_by_postprocessing.rs @@ -36,9 +36,9 @@ impl PostprocessingStep for OrderByPostprocessing { owned_table .inner_table() .get(&order_by.expr) - .ok_or(PostprocessingError::ColumnNotFound( - order_by.expr.to_string(), - ))? + .ok_or(PostprocessingError::ColumnNotFound { + column: order_by.expr.to_string(), + })? .clone(), order_by.direction, )) diff --git a/crates/proof-of-sql/src/sql/postprocessing/slice_postprocessing.rs b/crates/proof-of-sql/src/sql/postprocessing/slice_postprocessing.rs index 3556d9f82..3dd4c2731 100644 --- a/crates/proof-of-sql/src/sql/postprocessing/slice_postprocessing.rs +++ b/crates/proof-of-sql/src/sql/postprocessing/slice_postprocessing.rs @@ -44,10 +44,16 @@ impl PostprocessingStep for SlicePostprocessing { }; // The `possible_ending_row` is NOT inclusive. let possible_ending_row = (possible_starting_row + limit as i128).min(num_rows as i128); - let starting_row = usize::try_from(possible_starting_row) - .map_err(|_| PostprocessingError::InvalidSliceIndex(possible_starting_row))?; - let ending_row = usize::try_from(possible_ending_row) - .map_err(|_| PostprocessingError::InvalidSliceIndex(possible_ending_row))?; + let starting_row = usize::try_from(possible_starting_row).map_err(|_| { + PostprocessingError::InvalidSliceIndex { + index: possible_starting_row, + } + })?; + let ending_row = usize::try_from(possible_ending_row).map_err(|_| { + PostprocessingError::InvalidSliceIndex { + index: possible_ending_row, + } + })?; Ok(OwnedTable::::try_from_iter( owned_table .into_inner() diff --git a/crates/proof-of-sql/src/sql/proof/count_builder.rs b/crates/proof-of-sql/src/sql/proof/count_builder.rs index 1d974016d..7ed618d2b 100644 --- a/crates/proof-of-sql/src/sql/proof/count_builder.rs +++ b/crates/proof-of-sql/src/sql/proof/count_builder.rs @@ -24,9 +24,9 @@ impl<'a> CountBuilder<'a> { /// pass of verification. pub fn consume_bit_distribution(&mut self) -> Result { if self.bit_distributions.is_empty() { - Err(ProofError::VerificationError( - "expected prover to provide bit distribution", - )) + Err(ProofError::VerificationError { + error: "expected prover to provide bit distribution", + }) } else { let res = self.bit_distributions[0].clone(); self.bit_distributions = &self.bit_distributions[1..]; @@ -57,9 +57,9 @@ impl<'a> CountBuilder<'a> { pub fn counts(&self) -> Result { if !self.bit_distributions.is_empty() { - return Err(ProofError::VerificationError( - "incorrect number of bit distributions provided", - )); + return Err(ProofError::VerificationError { + error: "incorrect number of bit distributions provided", + }); } Ok(self.counts) } diff --git a/crates/proof-of-sql/src/sql/proof/query_proof.rs b/crates/proof-of-sql/src/sql/proof/query_proof.rs index 04be6d2e5..00f86b739 100644 --- a/crates/proof-of-sql/src/sql/proof/query_proof.rs +++ b/crates/proof-of-sql/src/sql/proof/query_proof.rs @@ -155,7 +155,9 @@ impl QueryProof { // validate bit decompositions for dist in self.bit_distributions.iter() { if !dist.is_valid() { - Err(ProofError::VerificationError("invalid bit distributions"))?; + Err(ProofError::VerificationError { + error: "invalid bit distributions", + })?; } } @@ -168,7 +170,9 @@ impl QueryProof { // verify sizes if !self.validate_sizes(&counts, result) { - Err(ProofError::VerificationError("invalid proof size"))?; + Err(ProofError::VerificationError { + error: "invalid proof size", + })?; } // construct a transcript for the proof @@ -254,9 +258,9 @@ impl QueryProof { // perform the evaluation check of the sumcheck polynomial if builder.sumcheck_evaluation() != subclaim.expected_evaluation { - Err(ProofError::VerificationError( - "sumcheck evaluation check failed", - ))?; + Err(ProofError::VerificationError { + error: "sumcheck evaluation check failed", + })?; } // finally, check the MLE evaluations with the inner product proof @@ -272,8 +276,8 @@ impl QueryProof { table_length, setup, ) - .map_err(|_e| { - ProofError::VerificationError("Inner product proof of MLE evaluations failed") + .map_err(|_e| ProofError::VerificationError { + error: "Inner product proof of MLE evaluations failed", })?; let mut verification_hash = [0u8; 32]; diff --git a/crates/proof-of-sql/src/sql/proof/query_result.rs b/crates/proof-of-sql/src/sql/proof/query_result.rs index f8432cb71..a0087839c 100644 --- a/crates/proof-of-sql/src/sql/proof/query_result.rs +++ b/crates/proof-of-sql/src/sql/proof/query_result.rs @@ -5,34 +5,40 @@ use crate::base::{ }; #[cfg(feature = "arrow")] use arrow::{error::ArrowError, record_batch::RecordBatch}; -use thiserror::Error; +use snafu::Snafu; /// Verifiable query errors -#[derive(Error, Debug)] +#[derive(Snafu, Debug)] pub enum QueryError { /// The query result overflowed. This does not mean that the verification failed. /// This just means that the database was supposed to respond with a result that was too large. - #[error("Overflow error")] + #[snafu(display("Overflow error"))] Overflow, /// The query result string could not be decoded. This does not mean that the verification failed. /// This just means that the database was supposed to respond with a string that was not valid UTF-8. - #[error("String decode error")] + #[snafu(display("String decode error"))] InvalidString, /// Decoding errors other than overflow and invalid string. - #[error("Miscellaneous decoding error")] + #[snafu(display("Miscellaneous decoding error"))] MiscellaneousDecodingError, /// Indexes are invalid. - #[error("Invalid indexes")] + #[snafu(display("Invalid indexes"))] InvalidIndexes, /// Miscellaneous evaluation error. - #[error("Miscellaneous evaluation error")] + #[snafu(display("Miscellaneous evaluation error"))] MiscellaneousEvaluationError, /// The proof failed to verify. - #[error(transparent)] - ProofError(#[from] ProofError), + #[snafu(transparent)] + ProofError { + /// The underlying source error + source: ProofError, + }, /// The table data was invalid. This should never happen because this should get caught by the verifier before reaching this point. - #[error(transparent)] - InvalidTable(#[from] OwnedTableError), + #[snafu(transparent)] + InvalidTable { + /// The underlying source error + source: OwnedTableError, + }, } /// The verified results of a query along with metadata produced by verification diff --git a/crates/proof-of-sql/src/sql/proof/verifiable_query_result.rs b/crates/proof-of-sql/src/sql/proof/verifiable_query_result.rs index 122584edb..8d0acb20a 100644 --- a/crates/proof-of-sql/src/sql/proof/verifiable_query_result.rs +++ b/crates/proof-of-sql/src/sql/proof/verifiable_query_result.rs @@ -120,9 +120,9 @@ impl VerifiableQueryResult { // handle the empty case if expr.is_empty(accessor) { if self.provable_result.is_some() || self.proof.is_some() { - return Err(ProofError::VerificationError( - "zero sumcheck variables but non-empty result", - ))?; + return Err(ProofError::VerificationError { + error: "zero sumcheck variables but non-empty result", + })?; } let result_fields = expr.get_column_result_fields(); @@ -131,9 +131,9 @@ impl VerifiableQueryResult { } if self.provable_result.is_none() || self.proof.is_none() { - return Err(ProofError::VerificationError( - "non-zero sumcheck variables but empty result", - ))?; + return Err(ProofError::VerificationError { + error: "non-zero sumcheck variables but empty result", + })?; } self.proof.as_ref().unwrap().verify( diff --git a/crates/proof-of-sql/src/sql/proof_exprs/add_subtract_expr_test.rs b/crates/proof-of-sql/src/sql/proof_exprs/add_subtract_expr_test.rs index fb3df21ad..c4f5f3327 100644 --- a/crates/proof-of-sql/src/sql/proof_exprs/add_subtract_expr_test.rs +++ b/crates/proof-of-sql/src/sql/proof_exprs/add_subtract_expr_test.rs @@ -107,7 +107,7 @@ fn decimal_column_type_issues_error_out_when_producing_provable_ast() { let accessor = OwnedTableTestAccessor::::new_from_table(t, data, 0, ()); assert!(matches!( DynProofExpr::try_new_add(column(t, "a", &accessor), const_bigint::(1)), - Err(ConversionError::DataTypeMismatch(..)) + Err(ConversionError::DataTypeMismatch { .. }) )); } diff --git a/crates/proof-of-sql/src/sql/proof_exprs/bitwise_verification.rs b/crates/proof-of-sql/src/sql/proof_exprs/bitwise_verification.rs index 1c72faef2..f8fb749f1 100644 --- a/crates/proof-of-sql/src/sql/proof_exprs/bitwise_verification.rs +++ b/crates/proof-of-sql/src/sql/proof_exprs/bitwise_verification.rs @@ -45,9 +45,9 @@ pub fn verify_constant_sign_decomposition( if lhs == rhs { Ok(()) } else { - Err(ProofError::VerificationError( - "constant sign bitwise decomposition is invalid", - )) + Err(ProofError::VerificationError { + error: "constant sign bitwise decomposition is invalid", + }) } } @@ -67,8 +67,8 @@ pub fn verify_constant_abs_decomposition( if S::from(dist.constant_part()) * t == eval { Ok(()) } else { - Err(ProofError::VerificationError( - "constant absolute bitwise decomposition is invalid", - )) + Err(ProofError::VerificationError { + error: "constant absolute bitwise decomposition is invalid", + }) } } diff --git a/crates/proof-of-sql/src/sql/proof_exprs/comparison_util.rs b/crates/proof-of-sql/src/sql/proof_exprs/comparison_util.rs index f783bad8d..7efe8ab87 100644 --- a/crates/proof-of-sql/src/sql/proof_exprs/comparison_util.rs +++ b/crates/proof-of-sql/src/sql/proof_exprs/comparison_util.rs @@ -40,7 +40,10 @@ pub(crate) fn scale_and_subtract<'a, S: Scalar>( let lhs_len = lhs.len(); let rhs_len = rhs.len(); if lhs_len != rhs_len { - return Err(ConversionError::DifferentColumnLength(lhs_len, rhs_len)); + return Err(ConversionError::DifferentColumnLength { + len_a: lhs_len, + len_b: rhs_len, + }); } let lhs_type = lhs.column_type(); let rhs_type = rhs.column_type(); @@ -50,10 +53,10 @@ pub(crate) fn scale_and_subtract<'a, S: Scalar>( BinaryOperator::LessThanOrEqual }; if !type_check_binary_operation(&lhs_type, &rhs_type, operator) { - return Err(ConversionError::DataTypeMismatch( - lhs_type.to_string(), - rhs_type.to_string(), - )); + return Err(ConversionError::DataTypeMismatch { + left_type: lhs_type.to_string(), + right_type: rhs_type.to_string(), + }); } let max_scale = std::cmp::max(lhs_scale, rhs_scale); let lhs_upscale = max_scale - lhs_scale; @@ -72,9 +75,11 @@ pub(crate) fn scale_and_subtract<'a, S: Scalar>( ); // Check if the precision is valid let _max_precision = Precision::new(max_precision_value).map_err(|_| { - ConversionError::DecimalConversionError(DecimalError::InvalidPrecision( - max_precision_value.to_string(), - )) + ConversionError::DecimalConversionError { + source: DecimalError::InvalidPrecision { + error: max_precision_value.to_string(), + }, + } })?; } unchecked_subtract_impl( diff --git a/crates/proof-of-sql/src/sql/proof_exprs/dyn_proof_expr.rs b/crates/proof-of-sql/src/sql/proof_exprs/dyn_proof_expr.rs index 58a423083..92fd5692e 100644 --- a/crates/proof-of-sql/src/sql/proof_exprs/dyn_proof_expr.rs +++ b/crates/proof-of-sql/src/sql/proof_exprs/dyn_proof_expr.rs @@ -74,10 +74,10 @@ impl DynProofExpr { let lhs_datatype = lhs.data_type(); let rhs_datatype = rhs.data_type(); if !type_check_binary_operation(&lhs_datatype, &rhs_datatype, BinaryOperator::Equal) { - Err(ConversionError::DataTypeMismatch( - lhs_datatype.to_string(), - rhs_datatype.to_string(), - )) + Err(ConversionError::DataTypeMismatch { + left_type: lhs_datatype.to_string(), + right_type: rhs_datatype.to_string(), + }) } else { Ok(Self::Equals(EqualsExpr::new(Box::new(lhs), Box::new(rhs)))) } @@ -95,10 +95,10 @@ impl DynProofExpr { &rhs_datatype, BinaryOperator::LessThanOrEqual, ) { - Err(ConversionError::DataTypeMismatch( - lhs_datatype.to_string(), - rhs_datatype.to_string(), - )) + Err(ConversionError::DataTypeMismatch { + left_type: lhs_datatype.to_string(), + right_type: rhs_datatype.to_string(), + }) } else { Ok(Self::Inequality(InequalityExpr::new( Box::new(lhs), @@ -113,10 +113,10 @@ impl DynProofExpr { let lhs_datatype = lhs.data_type(); let rhs_datatype = rhs.data_type(); if !type_check_binary_operation(&lhs_datatype, &rhs_datatype, BinaryOperator::Add) { - Err(ConversionError::DataTypeMismatch( - lhs_datatype.to_string(), - rhs_datatype.to_string(), - )) + Err(ConversionError::DataTypeMismatch { + left_type: lhs_datatype.to_string(), + right_type: rhs_datatype.to_string(), + }) } else { Ok(Self::AddSubtract(AddSubtractExpr::new( Box::new(lhs), @@ -131,10 +131,10 @@ impl DynProofExpr { let lhs_datatype = lhs.data_type(); let rhs_datatype = rhs.data_type(); if !type_check_binary_operation(&lhs_datatype, &rhs_datatype, BinaryOperator::Subtract) { - Err(ConversionError::DataTypeMismatch( - lhs_datatype.to_string(), - rhs_datatype.to_string(), - )) + Err(ConversionError::DataTypeMismatch { + left_type: lhs_datatype.to_string(), + right_type: rhs_datatype.to_string(), + }) } else { Ok(Self::AddSubtract(AddSubtractExpr::new( Box::new(lhs), @@ -149,10 +149,10 @@ impl DynProofExpr { let lhs_datatype = lhs.data_type(); let rhs_datatype = rhs.data_type(); if !type_check_binary_operation(&lhs_datatype, &rhs_datatype, BinaryOperator::Multiply) { - Err(ConversionError::DataTypeMismatch( - lhs_datatype.to_string(), - rhs_datatype.to_string(), - )) + Err(ConversionError::DataTypeMismatch { + left_type: lhs_datatype.to_string(), + right_type: rhs_datatype.to_string(), + }) } else { Ok(Self::Multiply(MultiplyExpr::new( Box::new(lhs), diff --git a/crates/proof-of-sql/src/sql/proof_exprs/inequality_expr_test.rs b/crates/proof-of-sql/src/sql/proof_exprs/inequality_expr_test.rs index af8c220db..4a305c979 100644 --- a/crates/proof-of-sql/src/sql/proof_exprs/inequality_expr_test.rs +++ b/crates/proof-of-sql/src/sql/proof_exprs/inequality_expr_test.rs @@ -304,7 +304,7 @@ fn we_cannot_compare_columns_filtering_on_extreme_decimal_values() { const_scalar::(Curve25519Scalar::ONE), false ), - Err(ConversionError::DataTypeMismatch(_, _)) + Err(ConversionError::DataTypeMismatch { .. }) )); } diff --git a/crates/proof-of-sql/src/sql/proof_exprs/multiply_expr_test.rs b/crates/proof-of-sql/src/sql/proof_exprs/multiply_expr_test.rs index 7379af1a7..882de6244 100644 --- a/crates/proof-of-sql/src/sql/proof_exprs/multiply_expr_test.rs +++ b/crates/proof-of-sql/src/sql/proof_exprs/multiply_expr_test.rs @@ -79,7 +79,7 @@ fn decimal_column_type_issues_error_out_when_producing_provable_ast() { column(t, "a", &accessor), const_bigint::(1) ), - Err(ConversionError::DataTypeMismatch(..)) + Err(ConversionError::DataTypeMismatch { .. }) )); } diff --git a/crates/proof-of-sql/src/sql/proof_exprs/sign_expr.rs b/crates/proof-of-sql/src/sql/proof_exprs/sign_expr.rs index 74909b558..f4d819dd9 100644 --- a/crates/proof-of-sql/src/sql/proof_exprs/sign_expr.rs +++ b/crates/proof-of-sql/src/sql/proof_exprs/sign_expr.rs @@ -20,9 +20,9 @@ use bumpalo::Bump; pub fn count_sign(builder: &mut CountBuilder) -> Result<(), ProofError> { let dist = builder.consume_bit_distribution()?; if !is_within_acceptable_range(&dist) { - return Err(ProofError::VerificationError( - "bit distribution outside of acceptable range", - )); + return Err(ProofError::VerificationError { + error: "bit distribution outside of acceptable range", + }); } if dist.num_varying_bits() == 0 { return Ok(()); diff --git a/crates/proof-of-sql/src/sql/proof_plans/filter_exec.rs b/crates/proof-of-sql/src/sql/proof_plans/filter_exec.rs index c7a183d85..745ea1c51 100644 --- a/crates/proof-of-sql/src/sql/proof_plans/filter_exec.rs +++ b/crates/proof-of-sql/src/sql/proof_plans/filter_exec.rs @@ -101,10 +101,11 @@ where .collect::, _>>()?, ); // 3. indexes - let indexes_eval = builder - .mle_evaluations - .result_indexes_evaluation - .ok_or(ProofError::VerificationError("invalid indexes"))?; + let indexes_eval = builder.mle_evaluations.result_indexes_evaluation.ok_or( + ProofError::VerificationError { + error: "invalid indexes", + }, + )?; // 4. filtered_columns let filtered_columns_evals = Vec::from_iter( repeat_with(|| builder.consume_result_mle()).take(self.aliased_results.len()), @@ -229,7 +230,11 @@ fn verify_filter( let chi_eval = match builder.mle_evaluations.result_indexes_evaluation { Some(eval) => eval, - None => return Err(ProofError::VerificationError("Result indexes not valid.")), + None => { + return Err(ProofError::VerificationError { + error: "Result indexes not valid.", + }) + } }; let c_fold_eval = alpha * one_eval + fold_vals(beta, &c_evals); diff --git a/crates/proof-of-sql/src/sql/proof_plans/filter_exec_test_dishonest_prover.rs b/crates/proof-of-sql/src/sql/proof_plans/filter_exec_test_dishonest_prover.rs index 14ef39996..1141296eb 100644 --- a/crates/proof-of-sql/src/sql/proof_plans/filter_exec_test_dishonest_prover.rs +++ b/crates/proof-of-sql/src/sql/proof_plans/filter_exec_test_dishonest_prover.rs @@ -147,6 +147,8 @@ fn we_fail_to_verify_a_basic_filter_with_a_dishonest_prover() { let res = VerifiableQueryResult::::new(&expr, &accessor, &()); assert!(matches!( res.verify(&expr, &accessor, &()), - Err(QueryError::ProofError(ProofError::VerificationError(_))) + Err(QueryError::ProofError { + source: ProofError::VerificationError { .. } + }) )); } diff --git a/crates/proof-of-sql/src/sql/proof_plans/group_by_exec.rs b/crates/proof-of-sql/src/sql/proof_plans/group_by_exec.rs index 267e4462a..692fdc863 100644 --- a/crates/proof-of-sql/src/sql/proof_plans/group_by_exec.rs +++ b/crates/proof-of-sql/src/sql/proof_plans/group_by_exec.rs @@ -119,10 +119,11 @@ impl ProofPlan for GroupByExec { .map(|aliased_expr| aliased_expr.expr.verifier_evaluate(builder, accessor)) .collect::, _>>()?; // 3. indexes - let indexes_eval = builder - .mle_evaluations - .result_indexes_evaluation - .ok_or(ProofError::VerificationError("invalid indexes"))?; + let indexes_eval = builder.mle_evaluations.result_indexes_evaluation.ok_or( + ProofError::VerificationError { + error: "invalid indexes", + }, + )?; // 4. filtered_columns let group_by_result_columns_evals = Vec::from_iter( @@ -153,15 +154,15 @@ impl ProofPlan for GroupByExec { .iter() .map(|col| table.inner_table().get(&col.column_id())) .collect::>>() - .ok_or(ProofError::VerificationError( - "Result does not all correct group by columns.", - ))?; + .ok_or(ProofError::VerificationError { + error: "Result does not all correct group by columns.", + })?; if (0..table.num_rows() - 1) .any(|i| compare_indexes_by_owned_columns(&cols, i, i + 1).is_ge()) { - Err(ProofError::VerificationError( - "Result of group by not ordered as expected.", - ))?; + Err(ProofError::VerificationError { + error: "Result of group by not ordered as expected.", + })?; } } None => todo!("GroupByExec currently only supported at top level of query plan."), diff --git a/crates/proof-of-sql/tests/integration_tests.rs b/crates/proof-of-sql/tests/integration_tests.rs index 1a3a6c3ab..0a1c9670f 100644 --- a/crates/proof-of-sql/tests/integration_tests.rs +++ b/crates/proof-of-sql/tests/integration_tests.rs @@ -431,7 +431,7 @@ fn decimal_type_issues_should_cause_provable_ast_to_fail() { "sxt".parse().unwrap(), &accessor, ), - Err(ConversionError::DataTypeMismatch(..)) + Err(ConversionError::DataTypeMismatch { .. }) )); }