Clippy, formatting, and tests

hasura · Dec 12, 2024 · 2246952 · 2246952
1 parent 9ec8354
commit 2246952
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 42 deletions.
diff --git a/crates/configuration/src/introspection.rs b/crates/configuration/src/introspection.rs
@@ -1,6 +1,6 @@
 //! Configuration and state for our connector.
-use serde::Deserialize;
 use ndc_models::TypeRepresentation;
+use serde::Deserialize;
 
 #[derive(Deserialize, Debug)]
 pub struct IntrospectStoredProcedureArgument {
@@ -80,7 +80,9 @@ pub fn map_type_representation(sql_type: &str) -> Option<TypeRepresentation> {
         "datetime" | "datetime2" | "smalldatetime" => Some(TypeRepresentation::Timestamp),
         "datetimeoffset" => Some(TypeRepresentation::TimestampTZ),
         "time" | "timestamp" => Some(TypeRepresentation::String),
-        "char" | "varchar" | "text" | "nchar" | "nvarchar" | "ntext" => Some(TypeRepresentation::String),
+        "char" | "varchar" | "text" | "nchar" | "nvarchar" | "ntext" => {
+            Some(TypeRepresentation::String)
+        }
         "binary" | "varbinary" | "image" => Some(TypeRepresentation::String),
         "uniqueidentifier" => Some(TypeRepresentation::UUID),
         "xml" => Some(TypeRepresentation::String),

diff --git a/...shots/mutation_tests__stored_procedures__execute_stored_procedure_with_relationships.snap b/...shots/mutation_tests__stored_procedures__execute_stored_procedure_with_relationships.snap
@@ -13,31 +13,31 @@ expression: result
             "rows": [
               {
                 "InvoiceId": 98,
-                "Total": 3.98
+                "Total": "3.98"
               },
               {
                 "InvoiceId": 121,
-                "Total": 3.96
+                "Total": "3.96"
               },
               {
                 "InvoiceId": 143,
-                "Total": 5.94
+                "Total": "5.94"
               },
               {
                 "InvoiceId": 195,
-                "Total": 0.99
+                "Total": "0.99"
               },
               {
                 "InvoiceId": 316,
-                "Total": 1.98
+                "Total": "1.98"
               },
               {
                 "InvoiceId": 327,
-                "Total": 13.86
+                "Total": "13.86"
               },
               {
                 "InvoiceId": 382,
-                "Total": 8.91
+                "Total": "8.91"
               }
             ]
           }

diff --git a/crates/query-engine/sql/src/sql/helpers.rs b/crates/query-engine/sql/src/sql/helpers.rs
@@ -50,22 +50,24 @@ pub fn make_column(
     table: TableReference,
     name: ColumnName,
     alias: ColumnAlias,
-    column_type: ScalarType,
+    column_type: &ScalarType,
 ) -> (ColumnAlias, Expression) {
     (
         alias,
-        cast_column(column_type, Expression::ColumnReference(ColumnReference::TableColumn { table, name })),
+        cast_column(
+            column_type,
+            Expression::ColumnReference(ColumnReference::TableColumn { table, name }),
+        ),
     )
 }
 
 /// Should we cast the column to a specific type?
-pub fn cast_column(column_type: ScalarType, expression: Expression) -> Expression {
+pub fn cast_column(column_type: &ScalarType, expression: Expression) -> Expression {
     match column_type.0.as_str() {
-        "bigint" | "decimal" | "numeric" | "money" | "smallmoney" =>
-            Expression::Cast {
-                expression: Box::new(expression),
-                r#type: ScalarType("varchar".to_string()),
-            },
+        "bigint" | "decimal" | "numeric" | "money" | "smallmoney" => Expression::Cast {
+            expression: Box::new(expression),
+            r#type: ScalarType("varchar".to_string()),
+        },
         _ => expression,
     }
 }

diff --git a/crates/query-engine/translation/src/translation/query/aggregates.rs b/crates/query-engine/translation/src/translation/query/aggregates.rs
@@ -2,8 +2,11 @@
 
 use indexmap::IndexMap;
 
-use crate::translation::{error::Error, helpers::{Env, CollectionOrProcedureInfo}};
-use query_engine_sql::sql::{self, helpers::cast_column, ast::ScalarType};
+use crate::translation::{
+    error::Error,
+    helpers::{CollectionOrProcedureInfo, Env},
+};
+use query_engine_sql::sql::{self, ast::ScalarType, helpers::cast_column};
 
 /// Translate any aggregates we should include in the query into our SQL AST.
 pub fn translate(
@@ -48,22 +51,25 @@ pub fn translate(
                         },
                     );
                     let column_type = match collection_info {
-                        CollectionOrProcedureInfo::Collection(ci) => {
-                            match ci {
-                                crate::translation::helpers::CollectionInfo::Table { info, .. } => {
-                                    info.columns.get(column.as_str()).map(|c| c.r#type.clone())
-                                },
-                                crate::translation::helpers::CollectionInfo::NativeQuery { info, .. } => {
-                                    info.columns.get(column.as_str()).map(|c| c.r#type.clone())
-                                },
-                                _ => None,
+                        CollectionOrProcedureInfo::Collection(ci) => match ci {
+                            crate::translation::helpers::CollectionInfo::Table { info, .. } => {
+                                info.columns.get(column.as_str()).map(|c| c.r#type.clone())
                             }
-                        }
-                        CollectionOrProcedureInfo::Procedure(_) => None
+                            crate::translation::helpers::CollectionInfo::NativeQuery {
+                                info,
+                                ..
+                            } => info.columns.get(column.as_str()).map(|c| c.r#type.clone()),
+                            _ => None,
+                        },
+                        CollectionOrProcedureInfo::Procedure(_) => None,
                     };
-                    let function_type = column_type.and_then(|column_type| env.metadata.aggregate_functions.0
-                        .get(&column_type)
-                        .and_then(|functions| functions.get(function.as_str())));
+                    let function_type = column_type.and_then(|column_type| {
+                        env.metadata
+                            .aggregate_functions
+                            .0
+                            .get(&column_type)
+                            .and_then(|functions| functions.get(function.as_str()))
+                    });
                     match function.as_str() {
                         "SUM" | "AVG" => sql::ast::Expression::Cast {
                             expression: Box::new(sql::ast::Expression::FunctionCall {
@@ -81,12 +87,12 @@ pub fn translate(
                                 args: vec![column_ref_expression],
                             };
                             match function_type.map(|f| f.return_type.clone()) {
-                                Some(scalar_type) => cast_column(ScalarType(scalar_type.0), expression),
-                                None => expression
+                                Some(scalar_type) => {
+                                    cast_column(&ScalarType(scalar_type.0), expression)
+                                }
+                                None => expression,
                             }
                         }
-
-
                     }
                 }
                 ndc_models::Aggregate::StarCount {} => {

diff --git a/crates/query-engine/translation/src/translation/query/mod.rs b/crates/query-engine/translation/src/translation/query/mod.rs
@@ -89,8 +89,14 @@ pub fn translate_query(
     )?;
 
     // translate aggregate select. if there are no fields, make this a None
-    let aggregate_select =
-        root::translate_aggregate_query(env, state, collection_info, current_table, from_clause, query)?;
+    let aggregate_select = root::translate_aggregate_query(
+        env,
+        state,
+        collection_info,
+        current_table,
+        from_clause,
+        query,
+    )?;
 
     match (row_select, aggregate_select) {
         (Some(rows), None) => Ok(sql::helpers::SelectSet::Rows(rows)),

diff --git a/crates/query-engine/translation/src/translation/query/root.rs b/crates/query-engine/translation/src/translation/query/root.rs
@@ -30,8 +30,12 @@ pub fn translate_aggregate_query(
         None => Ok(None),
         Some(aggregate_fields) => {
             // create all aggregate columns
-            let aggregate_columns =
-                aggregates::translate(&current_table.reference, aggregate_fields, collection_info, env)?;
+            let aggregate_columns = aggregates::translate(
+                &current_table.reference,
+                aggregate_fields,
+                collection_info,
+                env,
+            )?;
 
             // create the select clause and the joins, order by, where clauses.
             // We don't add the limit afterwards.
@@ -191,7 +195,7 @@ pub fn translate_rows_query(
                             current_table.reference.clone(),
                             column_info.name,
                             sql::helpers::make_column_alias(alias.to_string()),
-                            ScalarType(column_info.r#type.0),
+                            &ScalarType(column_info.r#type.0),
                         ))
                     }
                     ndc_models::Field::Relationship {