Skip to content

cast int64 and numeric to string by default #416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Apr 16, 2024
2 changes: 1 addition & 1 deletion crates/configuration/src/version3/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ fn base_type_representations() -> database::TypeRepresentations {
// This is not what we do now and is a breaking change.
// This will need to be changed in the future. In the meantime, we report
// The type representation to be json.
database::TypeRepresentation::Json,
database::TypeRepresentation::Int64,
),
(
database::ScalarType("numeric".to_string()),
Expand Down
99 changes: 73 additions & 26 deletions crates/query-engine/translation/src/translation/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub struct State {
}

#[derive(Debug)]
/// Used for generating a unique name for intermediate tables.
pub struct TableAliasIndex(pub u64);

#[derive(Debug)]
Expand Down Expand Up @@ -89,13 +90,17 @@ pub enum CollectionInfo<'env> {
}

#[derive(Debug)]
/// Metadata information about a specific collection.
pub enum CompositeTypeInfo<'env> {
/// Metadata information about a specific collection or composite type.
pub enum CollectionOrCompositeTypeInfo<'env> {
CollectionInfo(CollectionInfo<'env>),
CompositeTypeInfo {
name: String,
info: metadata::CompositeType,
},
CompositeTypeInfo(CompositeTypeInfo),
}

#[derive(Debug)]
/// Information about composite types.
pub struct CompositeTypeInfo {
pub name: String,
pub info: metadata::CompositeType,
}

impl<'request> Env<'request> {
Expand All @@ -114,22 +119,23 @@ impl<'request> Env<'request> {
}
}

/// Lookup a collection's information in the metadata.

pub fn lookup_composite_type(
/// Lookup collection or composite type.
pub fn lookup_collection_or_composite_type(
&self,
type_name: &'request str,
) -> Result<CompositeTypeInfo<'request>, Error> {
) -> Result<CollectionOrCompositeTypeInfo<'request>, Error> {
let it_is_a_collection = self.lookup_collection(type_name);

match it_is_a_collection {
Ok(collection_info) => Ok(CompositeTypeInfo::CollectionInfo(collection_info)),
Ok(collection_info) => Ok(CollectionOrCompositeTypeInfo::CollectionInfo(
collection_info,
)),
Err(Error::CollectionNotFound(_)) => {
let its_a_type = self.metadata.composite_types.0.get(type_name).map(|t| {
CompositeTypeInfo::CompositeTypeInfo {
CollectionOrCompositeTypeInfo::CompositeTypeInfo(CompositeTypeInfo {
name: t.name.clone(),
info: t.clone(),
}
})
});

its_a_type.ok_or(Error::CollectionNotFound(type_name.to_string()))
Expand All @@ -138,6 +144,21 @@ impl<'request> Env<'request> {
}
}

pub fn lookup_composite_type(&self, type_name: &str) -> Result<CompositeTypeInfo, Error> {
let its_a_type =
self.metadata
.composite_types
.0
.get(type_name)
.map(|t| CompositeTypeInfo {
name: t.name.clone(),
info: t.clone(),
});

its_a_type.ok_or(Error::CollectionNotFound(type_name.to_string()))
}

/// Lookup a collection's information in the metadata.
pub fn lookup_collection(
&self,
collection_name: &'request str,
Expand Down Expand Up @@ -202,6 +223,23 @@ impl<'request> Env<'request> {
})
}

/// Lookup type representation of a type.
pub fn lookup_type_representation(
&self,
typ: &metadata::Type,
) -> Option<metadata::TypeRepresentation> {
match typ {
metadata::Type::ScalarType(scalar_type) => self
.metadata
.type_representations
.0
.get(scalar_type)
.cloned(),
metadata::Type::ArrayType(_) => None,
metadata::Type::CompositeType(_) => None,
}
}

/// Try to get the variables table reference. This will fail if no variables were passed
/// as part of the query request.
pub fn get_variables_table(&self) -> Result<sql::ast::TableReference, Error> {
Expand Down Expand Up @@ -242,28 +280,37 @@ impl CollectionInfo<'_> {
}
}

impl CompositeTypeInfo<'_> {
impl CollectionOrCompositeTypeInfo<'_> {
/// Lookup a column in a collection.
pub fn lookup_column(&self, column_name: &str) -> Result<ColumnInfo, Error> {
match self {
CompositeTypeInfo::CollectionInfo(collection_info) => {
CollectionOrCompositeTypeInfo::CollectionInfo(collection_info) => {
collection_info.lookup_column(column_name)
}
CompositeTypeInfo::CompositeTypeInfo { name, info } => info
.fields
.get(column_name)
.map(|field_info| ColumnInfo {
name: sql::ast::ColumnName(field_info.name.clone()),
r#type: field_info.r#type.clone(),
})
.ok_or(Error::ColumnNotFoundInCollection(
column_name.to_string(),
name.clone(),
)),
CollectionOrCompositeTypeInfo::CompositeTypeInfo(composite_type) => {
composite_type.lookup_column(column_name)
}
}
}
}

impl CompositeTypeInfo {
/// Lookup a column in a composite type.
pub fn lookup_column(&self, column_name: &str) -> Result<ColumnInfo, Error> {
self.info
.fields
.get(column_name)
.map(|field_info| ColumnInfo {
name: sql::ast::ColumnName(field_info.name.clone()),
r#type: field_info.r#type.clone(),
})
.ok_or(Error::ColumnNotFoundInCollection(
column_name.to_string(),
self.name.clone(),
))
}
}

impl Default for State {
fn default() -> State {
State {
Expand Down
159 changes: 148 additions & 11 deletions crates/query-engine/translation/src/translation/query/fields.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
//! Handle 'rows' and 'aggregates' translation.

use indexmap::indexmap;
use indexmap::IndexMap;

use ndc_sdk::models;

use super::relationships;
use crate::translation::error::Error;
use crate::translation::helpers::{ColumnInfo, Env, State, TableNameAndReference};
use query_engine_metadata::metadata::Type;
use crate::translation::helpers::{
CollectionOrCompositeTypeInfo, ColumnInfo, Env, State, TableNameAndReference,
};
use query_engine_metadata::metadata::{Type, TypeRepresentation};
use query_engine_sql::sql;

/// This type collects the salient parts of joined-on subqueries that compute the result of a
Expand Down Expand Up @@ -41,7 +44,7 @@ pub(crate) fn translate_fields(
join_relationship_fields: &mut Vec<relationships::JoinFieldInfo>,
) -> Result<sql::ast::Select, Error> {
// find the table according to the metadata.
let type_info = env.lookup_composite_type(&current_table.name)?;
let type_info = env.lookup_collection_or_composite_type(&current_table.name)?;

// Each nested field is computed in one joined-on sub query.
let mut nested_field_joins: Vec<JoinNestedFieldInfo> = vec![];
Expand All @@ -52,14 +55,16 @@ pub(crate) fn translate_fields(
models::Field::Column {
column,
fields: None,
} => {
let column_info = type_info.lookup_column(&column)?;
Ok(sql::helpers::make_column(
current_table.reference.clone(),
column_info.name.clone(),
sql::helpers::make_column_alias(alias),
))
}
} => unpack_and_wrap_fields(
env,
state,
current_table,
join_relationship_fields,
&column,
sql::helpers::make_column_alias(alias),
&type_info,
&mut nested_field_joins,
),
models::Field::Column {
column,
fields: Some(nested_field),
Expand Down Expand Up @@ -318,3 +323,135 @@ fn translate_nested_field(
},
))
}

#[allow(clippy::too_many_arguments)]
/// In order to return the expected type representation for each column,
/// we need to wrap columns in type representation cast, and unpack composite types
/// so we can wrap them.
fn unpack_and_wrap_fields(
env: &Env,
state: &mut State,
current_table: &TableNameAndReference,
join_relationship_fields: &mut Vec<relationships::JoinFieldInfo>,
column: &str,
alias: sql::ast::ColumnAlias,
type_info: &CollectionOrCompositeTypeInfo<'_>,
nested_field_joins: &mut Vec<JoinNestedFieldInfo>,
) -> Result<(sql::ast::ColumnAlias, sql::ast::Expression), Error> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that I would be partial to inlining this function in the one place where it is used, since its body has to take part in the somewhat complex join_relationship_fields/nested_field_joins handling.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually pulled it out because that function has gotten quite big and found this to be more digestible, but we can talk about this if you really prefer it to be inlined.

let column_info = type_info.lookup_column(column)?;

// Different kinds of types have different strategy for converting to their
// type representation.
match column_info.r#type {
// Scalar types can just be wrapped in a cast.
Type::ScalarType(_) => {
let column_type_representation = env.lookup_type_representation(&column_info.r#type);
let (alias, expression) = sql::helpers::make_column(
current_table.reference.clone(),
column_info.name.clone(),
alias,
);
Ok((
alias,
wrap_in_type_representation(expression, column_type_representation),
))
}
// Composite types are a more involved case because we cannot just "cast"
// a composite type, we need to unpack it and cast the individual fields.
// In this case, we unpack a single composite column selection to an explicit
// selection of all fields.
Type::CompositeType(ref composite_type) => {
// build a nested field selection of all fields.
let nested_field = models::NestedField::Object({
let composite_type = env.lookup_composite_type(composite_type)?;
let mut fields = indexmap!();
for (name, field_info) in composite_type.info.fields.iter() {
fields.insert(
name.to_string(),
models::Field::Column {
column: field_info.name.clone(),
fields: None,
},
);
}
models::NestedObject { fields }
});

// translate this as if it is a nested field selection.
let (nested_field_join, nested_column_reference) = translate_nested_field(
env,
state,
current_table,
&column_info,
nested_field,
join_relationship_fields,
)?;

nested_field_joins.push(nested_field_join);

Ok((
alias,
sql::ast::Expression::ColumnReference(nested_column_reference),
))
}
// TODO: Arrays for composite types and nested arrays are not handled yet.
Type::ArrayType(type_boxed) => {
let inner_column_type_representation = env.lookup_type_representation(&*type_boxed);
let (alias, expression) = sql::helpers::make_column(
current_table.reference.clone(),
column_info.name.clone(),
alias,
);
Ok((
alias,
wrap_array_in_type_representation(expression, inner_column_type_representation),
))
}
}
}

/// Certain type representations require that we provide a different json representation
/// than what postgres will return.
/// For array columns of those type representation, we wrap the result in a cast.
fn wrap_array_in_type_representation(
expression: sql::ast::Expression,
inner_column_type_representation: Option<TypeRepresentation>,
) -> sql::ast::Expression {
match inner_column_type_representation {
None => expression,
Some(type_rep) => match type_rep {
TypeRepresentation::Int64 => sql::ast::Expression::Cast {
expression: Box::new(expression),
r#type: sql::ast::ScalarType("text[]".to_string()),
},
TypeRepresentation::BigDecimal => sql::ast::Expression::Cast {
expression: Box::new(expression),
r#type: sql::ast::ScalarType("text[]".to_string()),
},
_ => expression,
},
}
}

/// Certain type representations require that we provide a different json representation
/// than what postgres will return.
/// For columns of those type representation, we wrap the result in a cast.
fn wrap_in_type_representation(
expression: sql::ast::Expression,
column_type_representation: Option<TypeRepresentation>,
) -> sql::ast::Expression {
match column_type_representation {
None => expression,
Some(type_rep) => match type_rep {
TypeRepresentation::Int64 => sql::ast::Expression::Cast {
expression: Box::new(expression),
r#type: sql::ast::ScalarType("text".to_string()),
},
TypeRepresentation::BigDecimal => sql::ast::Expression::Cast {
expression: Box::new(expression),
r#type: sql::ast::ScalarType("text".to_string()),
},
_ => expression,
},
}
}
Loading