Skip to content

Commit

Permalink
Properly cast column types for spec
Browse files Browse the repository at this point in the history
  • Loading branch information
codedmart committed Dec 12, 2024
1 parent 8b71c9b commit 9ec8354
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 20 deletions.
8 changes: 4 additions & 4 deletions crates/configuration/src/introspection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,17 @@ pub fn map_type_representation(sql_type: &str) -> Option<TypeRepresentation> {
"float" => Some(TypeRepresentation::Float64),
"real" => Some(TypeRepresentation::Float32),
"date" => Some(TypeRepresentation::Date),
"datetime" | "datetime2" | "smalldatetime" | "timestamp" => Some(TypeRepresentation::Timestamp),
"datetime" | "datetime2" | "smalldatetime" => Some(TypeRepresentation::Timestamp),

Check warning on line 80 in crates/configuration/src/introspection.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/ndc-sqlserver/ndc-sqlserver/crates/configuration/src/introspection.rs
"datetimeoffset" => Some(TypeRepresentation::TimestampTZ),
"time" => Some(TypeRepresentation::String),
"time" | "timestamp" => 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),
"geometry" => Some(TypeRepresentation::Geometry),
"geography" => Some(TypeRepresentation::Geography),
"json" => Some(TypeRepresentation::JSON),
// TODO: Add support for hierarchyid and sql_variant
// "geometry" => Some(TypeRepresentation::Geometry),
// "geography" => Some(TypeRepresentation::Geography),
// "hierarchyid" | "sql_variant" => XXX,
_ => None,
}
Expand Down
16 changes: 15 additions & 1 deletion crates/query-engine/sql/src/sql/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,26 @@ pub fn make_column(
table: TableReference,
name: ColumnName,
alias: ColumnAlias,
column_type: ScalarType,
) -> (ColumnAlias, Expression) {

Check warning on line 54 in crates/query-engine/sql/src/sql/helpers.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/ndc-sqlserver/ndc-sqlserver/crates/query-engine/sql/src/sql/helpers.rs
(
alias,
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?

Check warning on line 61 in crates/query-engine/sql/src/sql/helpers.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/ndc-sqlserver/ndc-sqlserver/crates/query-engine/sql/src/sql/helpers.rs
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()),
},
_ => expression,
}
}

/// Create column aliases using this function so we build everything in one place.
pub fn make_column_alias(name: String) -> ColumnAlias {
ColumnAlias { name }
Expand Down
2 changes: 1 addition & 1 deletion crates/query-engine/translation/src/translation/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use query_engine_sql::sql;
#[derive(Debug)]
/// Static information from the query and metadata.
pub struct Env<'a> {
metadata: &'a metadata::Metadata,
pub metadata: &'a metadata::Metadata,
relationships: BTreeMap<ndc_models::RelationshipName, ndc_models::Relationship>,
}

Expand Down
54 changes: 42 additions & 12 deletions crates/query-engine/translation/src/translation/query/aggregates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

Check warning on line 2 in crates/query-engine/translation/src/translation/query/aggregates.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/ndc-sqlserver/ndc-sqlserver/crates/query-engine/translation/src/translation/query/aggregates.rs
use indexmap::IndexMap;

use crate::translation::error::Error;
use query_engine_sql::sql;
use crate::translation::{error::Error, helpers::{Env, CollectionOrProcedureInfo}};
use query_engine_sql::sql::{self, helpers::cast_column, ast::ScalarType};

/// Translate any aggregates we should include in the query into our SQL AST.
pub fn translate(
table: &sql::ast::TableReference,
aggregates: &IndexMap<ndc_models::FieldName, ndc_models::Aggregate>,
collection_info: &CollectionOrProcedureInfo,
env: &Env,
) -> Result<Vec<(sql::ast::ColumnAlias, sql::ast::Expression)>, Error> {
aggregates
.into_iter()
Expand Down Expand Up @@ -45,18 +47,46 @@ pub fn translate(
column: sql::helpers::make_column_alias(column.to_string()),
},

Check warning on line 48 in crates/query-engine/translation/src/translation/query/aggregates.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/ndc-sqlserver/ndc-sqlserver/crates/query-engine/translation/src/translation/query/aggregates.rs
);
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::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())));
match function.as_str() {
"SUM" | "AVG" => sql::ast::Expression::FunctionCall {
function: sql::ast::Function::Unknown(function.to_string()),
args: vec![sql::ast::Expression::Cast {
expression: Box::new(column_ref_expression),
r#type: sql::ast::ScalarType("BIGINT".to_string()),
}],
},
_ => sql::ast::Expression::FunctionCall {
function: sql::ast::Function::Unknown(function.to_string()),
args: vec![column_ref_expression],
"SUM" | "AVG" => sql::ast::Expression::Cast {
expression: Box::new(sql::ast::Expression::FunctionCall {
function: sql::ast::Function::Unknown(function.to_string()),
args: vec![sql::ast::Expression::Cast {
expression: Box::new(column_ref_expression),
r#type: sql::ast::ScalarType("BIGINT".to_string()),
}],
}),
r#type: sql::ast::ScalarType("varchar".to_string()),
},
_ => {
let expression = sql::ast::Expression::FunctionCall {
function: sql::ast::Function::Unknown(function.to_string()),
args: vec![column_ref_expression],

Check warning on line 81 in crates/query-engine/translation/src/translation/query/aggregates.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/ndc-sqlserver/ndc-sqlserver/crates/query-engine/translation/src/translation/query/aggregates.rs
};
match function_type.map(|f| f.return_type.clone()) {
Some(scalar_type) => cast_column(ScalarType(scalar_type.0), expression),
None => expression
}
}


}
}
ndc_models::Aggregate::StarCount {} => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ 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, current_table, from_clause, query)?;
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)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::translation::helpers::CollectionOrProcedureInfo;
use crate::translation::helpers::{
CollectionInfo, Env, RootAndCurrentTables, State, TableNameAndReference,
};
use query_engine_sql::sql::ast::ScalarType;

use super::relationships;
use super::sorting;
Expand All @@ -20,6 +21,7 @@ use query_engine_sql::sql;
pub fn translate_aggregate_query(
env: &Env,
state: &mut State,
collection_info: &CollectionOrProcedureInfo,
current_table: &TableNameAndReference,
from_clause: &sql::ast::From,
query: &ndc_models::Query,
Expand All @@ -29,7 +31,7 @@ pub fn translate_aggregate_query(
Some(aggregate_fields) => {
// create all aggregate columns
let aggregate_columns =
aggregates::translate(&current_table.reference, aggregate_fields)?;
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.
Expand Down Expand Up @@ -189,6 +191,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),
))
}
ndc_models::Field::Relationship {
Expand Down

0 comments on commit 9ec8354

Please sign in to comment.