fix panic

spaceandtimelabs · Dec 16, 2024 · 22bbcfc · 22bbcfc
1 parent 2e6b0f3
commit 22bbcfc
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 27 deletions.
diff --git a/crates/proof-of-sql/src/evm_compatibility/dyn_proof_plan_serializer/serialize_proof_expr.rs b/crates/proof-of-sql/src/evm_compatibility/dyn_proof_plan_serializer/serialize_proof_expr.rs
@@ -80,14 +80,14 @@ mod tests {
     fn we_can_serialize_a_column_expr() {
         let table_ref: TableRef = "namespace.table".parse().unwrap();
         let column_0_ref: ColumnRef =
-            ColumnRef::new(table_ref, "column_0".parse().unwrap(), ColumnType::BigInt);
+            ColumnRef::new(table_ref, "column_0".into(), ColumnType::BigInt);
         let column_1_ref: ColumnRef =
-            ColumnRef::new(table_ref, "column_1".parse().unwrap(), ColumnType::BigInt);
+            ColumnRef::new(table_ref, "column_1".into(), ColumnType::BigInt);
         let column_2_ref: ColumnRef =
-            ColumnRef::new(table_ref, "column_2".parse().unwrap(), ColumnType::BigInt);
+            ColumnRef::new(table_ref, "column_2".into(), ColumnType::BigInt);
         let serializer = DynProofPlanSerializer::<TestScalar>::try_new(
             indexset! {},
-            indexset! { column_0_ref, column_1_ref },
+            indexset! { column_0_ref.clone(), column_1_ref.clone() },
         )
         .unwrap();
 
@@ -178,10 +178,12 @@ mod tests {
     fn we_can_serialize_an_equals_expr() {
         let table_ref: TableRef = "namespace.table".parse().unwrap();
         let column_0_ref: ColumnRef =
-            ColumnRef::new(table_ref, "column_0".parse().unwrap(), ColumnType::BigInt);
-        let serializer =
-            DynProofPlanSerializer::<TestScalar>::try_new(indexset! {}, indexset! { column_0_ref })
-                .unwrap();
+            ColumnRef::new(table_ref, "column_0".into(), ColumnType::BigInt);
+        let serializer = DynProofPlanSerializer::<TestScalar>::try_new(
+            indexset! {},
+            indexset! { column_0_ref.clone() },
+        )
+        .unwrap();
 
         let lhs = DynProofExpr::Column(ColumnExpr::new(column_0_ref));
         let rhs = DynProofExpr::Literal(LiteralExpr::new(LiteralValue::BigInt(4200)));
@@ -229,10 +231,12 @@ mod tests {
     fn we_cannot_serialize_an_unsupported_expr() {
         let table_ref: TableRef = "namespace.table".parse().unwrap();
         let column_0_ref: ColumnRef =
-            ColumnRef::new(table_ref, "column_0".parse().unwrap(), ColumnType::BigInt);
-        let serializer =
-            DynProofPlanSerializer::<TestScalar>::try_new(indexset! {}, indexset! { column_0_ref })
-                .unwrap();
+            ColumnRef::new(table_ref, "column_0".into(), ColumnType::BigInt);
+        let serializer = DynProofPlanSerializer::<TestScalar>::try_new(
+            indexset! {},
+            indexset! { column_0_ref.clone() },
+        )
+        .unwrap();
 
         let lhs = DynProofExpr::Column(ColumnExpr::new(column_0_ref));
         let rhs = DynProofExpr::Literal(LiteralExpr::new(LiteralValue::BigInt(4200)));

diff --git a/crates/proof-of-sql/src/evm_compatibility/dyn_proof_plan_serializer/serialize_proof_plan.rs b/crates/proof-of-sql/src/evm_compatibility/dyn_proof_plan_serializer/serialize_proof_plan.rs
@@ -89,7 +89,7 @@ mod tests {
 
         let aliased_expr = AliasedDynProofExpr {
             expr: expr.clone(),
-            alias: "alias".parse().unwrap(),
+            alias: "alias".into(),
         };
         let bytes = serializer
             .clone()
@@ -155,11 +155,11 @@ mod tests {
         let expr_c = DynProofExpr::Literal(LiteralExpr::new(LiteralValue::BigInt(4200)));
         let aliased_expr_0 = AliasedDynProofExpr {
             expr: expr_a.clone(),
-            alias: "alias_0".parse().unwrap(),
+            alias: "alias_0".into(),
         };
         let aliased_expr_1 = AliasedDynProofExpr {
             expr: expr_b.clone(),
-            alias: "alias_1".parse().unwrap(),
+            alias: "alias_1".into(),
         };
         let table_expr = TableExpr { table_ref };
 

diff --git a/crates/proof-of-sql/src/evm_compatibility/dyn_proof_plan_serializer/serializer.rs b/crates/proof-of-sql/src/evm_compatibility/dyn_proof_plan_serializer/serializer.rs
@@ -87,12 +87,12 @@ mod tests {
         let table_ref_1: TableRef = "namespace.table1".parse().unwrap();
         let table_ref_2: TableRef = "namespace.table2".parse().unwrap();
         let column_ref_1: ColumnRef =
-            ColumnRef::new(table_ref_1, "column1".parse().unwrap(), ColumnType::BigInt);
+            ColumnRef::new(table_ref_1, "column1".into(), ColumnType::BigInt);
         let column_ref_2: ColumnRef =
-            ColumnRef::new(table_ref_2, "column2".parse().unwrap(), ColumnType::BigInt);
+            ColumnRef::new(table_ref_2, "column2".into(), ColumnType::BigInt);
 
         let table_refs = indexset! { table_ref_1, table_ref_2 };
-        let column_refs = indexset! { column_ref_1, column_ref_2 };
+        let column_refs = indexset! { column_ref_1.clone(), column_ref_2.clone() };
         let serializer =
             DynProofPlanSerializer::<TestScalar>::try_new(table_refs, column_refs).unwrap();
         assert_eq!(
@@ -125,7 +125,7 @@ mod tests {
             .map(|i| {
                 ColumnRef::new(
                     table_ref,
-                    format!("column{i}").parse().unwrap(),
+                    format!("column{i}").as_str().into(),
                     ColumnType::BigInt,
                 )
             })

diff --git a/crates/proof-of-sql/src/evm_compatibility/serialize_query_expr.rs b/crates/proof-of-sql/src/evm_compatibility/serialize_query_expr.rs
@@ -70,7 +70,7 @@ mod tests {
     #[test]
     fn we_can_generate_serialized_proof_plan_for_query_expr() {
         let table_ref = "namespace.table".parse().unwrap();
-        let identifier_alias = "alias".parse().unwrap();
+        let identifier_alias = "alias".into();
 
         let plan = DynProofPlan::Filter(FilterExec::new(
             vec![AliasedDynProofExpr {
@@ -121,9 +121,9 @@ mod tests {
     #[test]
     fn we_can_generate_serialized_proof_plan_for_simple_filter() {
         let table_ref = "namespace.table".parse().unwrap();
-        let identifier_a = "a".parse().unwrap();
-        let identifier_b = "b".parse().unwrap();
-        let identifier_alias = "alias".parse().unwrap();
+        let identifier_a = "a".into();
+        let identifier_b = "b".into();
+        let identifier_alias = "alias".into();
 
         let column_ref_a = ColumnRef::new(table_ref, identifier_a, ColumnType::BigInt);
         let column_ref_b = ColumnRef::new(table_ref, identifier_b, ColumnType::BigInt);

diff --git a/crates/proof-of-sql/src/sql/parse/error.rs b/crates/proof-of-sql/src/sql/parse/error.rs
@@ -147,6 +147,12 @@ pub enum ConversionError {
         /// The operator that is unsupported
         message: String,
     },
+    /// Errors in converting `Ident` to `Identifier`
+    #[snafu(display("Failed to convert `Ident` to `Identifier`: {error}"))]
+    IdentifierConversionError {
+        /// The underlying error message
+        error: String,
+    },
 }
 
 impl From<String> for 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
@@ -114,12 +114,13 @@ impl<'a> QueryContextBuilder<'a> {
         columns
     }
 
-    /// # Panics
-    /// This function will panic if the conversion from `Ident` to `Identifier` fails.
     fn visit_select_all_expr(&mut self) -> ConversionResult<()> {
         for (column_name, _) in self.lookup_schema() {
-            let column_identifier =
-                Identifier::try_from(column_name).expect("Failed to convert Ident to Identifier");
+            let column_identifier = Identifier::try_from(column_name).map_err(|e| {
+                ConversionError::IdentifierConversionError {
+                    error: format!("Failed to convert Ident to Identifier: {e}"),
+                }
+            })?;
             let col_expr = Expression::Column(column_identifier);
             self.visit_aliased_expr(AliasedResultExpr::new(col_expr, column_identifier))?;
         }