Skip to content

Commit

Permalink
Unify code path for json value translation with variables (#267)
Browse files Browse the repository at this point in the history
### What

Rather than having two different ways to translate values (depending on
whether they are literals or variables) we instead use the same method
in both cases.

### How

As a consequence we need to have the `State` available in literal value
translation as well as in variable translation, which requires some
follow-up changes in the various other translation functions.

We also duplicate the test of nested array types from the variables-case
to the literal values-case.
  • Loading branch information
plcplc committed Jan 23, 2024
1 parent 3d19e81 commit 9eec086
Show file tree
Hide file tree
Showing 16 changed files with 401 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub fn translate_delete(
.get(&by_column.name)
.ok_or(Error::ArgumentNotFound(by_column.name.clone()))?;

let key_value = translate_json_value(unique_key, &by_column.r#type).unwrap();
let key_value = translate_json_value(state, unique_key, &by_column.r#type).unwrap();

let table = ast::TableReference::DBTable {
schema: schema_name.clone(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub fn generate(
/// Given the description of an insert mutation (ie, `InsertMutation`),
/// and the arguments, output the SQL AST.
pub fn translate(
// state: &mut crate::translation::helpers::State,
state: &mut crate::translation::helpers::State,
mutation: &InsertMutation,
arguments: BTreeMap<String, serde_json::Value>,
) -> Result<ast::Insert, Error> {
Expand All @@ -64,7 +64,7 @@ pub fn translate(
))?;

columns.push(ast::ColumnName(column_info.name.clone()));
values.push(translate_json_value(value, &column_info.r#type)?);
values.push(translate_json_value(state, value, &column_info.r#type)?);
}
}
_ => todo!(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ fn translate_mutation(
let return_collection = insert.collection_name.clone();
(
return_collection,
sql::ast::CTExpr::Insert(mutation::insert::translate(&insert, arguments)?),
sql::ast::CTExpr::Insert(mutation::insert::translate(
&mut state, &insert, arguments,
)?),
)
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,11 @@ fn translate_comparison_value(
translate_comparison_target(env, state, root_and_current_tables, &column)
}
models::ComparisonValue::Scalar { value: json_value } => Ok((
values::translate_json_value(&json_value, &database::Type::ScalarType(typ.clone()))?,
values::translate_json_value(
state,
&json_value,
&database::Type::ScalarType(typ.clone()),
)?,
vec![],
)),
models::ComparisonValue::Variable { name: var } => Ok((
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,17 @@ pub fn translate(env: &Env, state: State) -> Result<Vec<sql::ast::CommonTableExp
let variables_table = env.get_variables_table();
let native_queries = state.get_native_queries();

// We need a 'State' value when translating variables in order
// to be able to generate fresh names for bound relational
// expressions.
// However, we cannot readily re-use the 'state' argument we're
// given, since that is an "owned" value which is "moved" in
// the course of iterating over the native queries accumulated
// within.
// Ideally we should resolve this by tracking native queries
// separately from the fresh table name counter.
let mut translation_state = State::new();

// for each found table expression
for native_query in native_queries {
// convert metadata representation to sql::ast representation
Expand All @@ -33,21 +44,12 @@ pub fn translate(env: &Env, state: State) -> Result<Vec<sql::ast::CommonTableExp
None => Err(Error::ArgumentNotFound(param.clone())),
Some(argument) => match argument {
models::Argument::Literal { value } => {
values::translate_json_value(value, &typ)
values::translate_json_value(&mut translation_state, value, &typ)
}
models::Argument::Variable { name } => match &variables_table {
Err(err) => Err(err.clone()),
Ok(variables_table) => Ok(values::translate_variable(
// We need a 'State' value when translating variables in order
// to be able to generate fresh names for bound relational
// expressions.
// However, we cannot readily re-use the 'state' argument we're
// given, since that is an "owned" value which is "moved" in
// the course of iterating over the native queries accumulated
// within.
// Ideally we should resolve this by tracking native queries
// separately from the fresh table name counter.
&mut State::new(),
&mut translation_state,
variables_table.clone(),
name.clone(),
&typ,
Expand Down
39 changes: 16 additions & 23 deletions crates/query-engine/translation/src/translation/query/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use sql::ast::{Expression, Value};

/// Convert a JSON value into a SQL value.
pub fn translate_json_value(
state: &mut State,
value: &serde_json::Value,
r#type: &database::Type,
) -> Result<sql::ast::Expression, Error> {
Expand All @@ -26,31 +27,23 @@ pub fn translate_json_value(
expression: Box::new(Expression::Value(Value::String(str.clone()))),
r#type: type_to_ast_scalar_type(r#type),
}),
(serde_json::Value::Array(arr), database::Type::ArrayType(element_type)) => {
let mut x: Vec<sql::ast::Expression> = vec![];

for element in arr {
x.push(translate_json_value(element, element_type)?)
}

Ok(Expression::Cast {
expression: Box::new(Expression::ArrayConstructor(x)),
r#type: type_to_ast_scalar_type(r#type),
})
(serde_json::Value::Array(_), database::Type::ArrayType(_)) => {
let value_expression =
sql::ast::Expression::Value(sql::ast::Value::JsonValue(value.clone()));
Ok(translate_projected_variable(
state,
r#type,
value_expression,
))
}
// For composite types we use the `jsonb_populate_record` function to build the composite
// value rather than constructing the value field-by-field in SQL.
(serde_json::Value::Object(_obj), database::Type::CompositeType(_type_name)) => {
Ok(sql::ast::Expression::FunctionCall {
function: sql::ast::Function::JsonbPopulateRecord,
args: vec![
sql::ast::Expression::Cast {
expression: Box::new(sql::ast::Expression::Value(sql::ast::Value::Null)),
r#type: type_to_ast_scalar_type(r#type),
},
Expression::Value(Value::JsonValue(value.clone())),
],
})
let value_expression =
sql::ast::Expression::Value(sql::ast::Value::JsonValue(value.clone()));
Ok(translate_projected_variable(
state,
r#type,
value_expression,
))
}
// If the type is not congruent with the value constructor we simply pass the json value
// raw and cast to the specified type. This allows users to consume any json values,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
{
"collection": "summarize_organizations",
"query": {
"fields": {
"result": {
"type": "column",
"column": "result",
"arguments": {}
}
}
},
"arguments": {
"organizations": {
"type": "literal",
"value": [
{
"name": "RC Model Airplane Enthusiasts",
"committees": [
{
"name": "Founders",
"members": [
{
"first_name": "Orville",
"last_name": "Wright"
},
{
"first_name": "Wilbur",
"last_name": "Wright"
}
]
},
{
"name": "Parts supply management",
"members": [
{
"first_name": "Orville",
"last_name": "Wright"
},
{
"first_name": "Wilbur",
"last_name": "Wright"
},
{
"first_name": "Guybrush",
"last_name": "Threepwood"
}
]
}
]
},
{
"name": "Argonauts' Alumni Association",
"committees": [
{
"name": "Crew",
"members": [
{
"first_name": "Jason",
"last_name": "(The)"
},
{
"first_name": "Heracles",
"last_name": "(The)"
},
{
"first_name": "Castor",
"last_name": "(The)"
},
{
"first_name": "Polydeuces",
"last_name": "(The)"
}
]
}
]
}
]
}
},
"collection_relationships": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
{
"compositeTypes": {
"person_name": {
"name": "person_name",
"fields": {
"first_name": {
"name": "first_name",
"type": {
"scalarType": "text"
}
},
"last_name": {
"name": "last_name",
"type": {
"scalarType": "text"
}
}
}
},
"committee": {
"name": "committee",
"fields": {
"name": {
"name": "name",
"type": {
"scalarType": "text"
}
},
"members": {
"name": "members",
"type": {
"arrayType": {
"scalarType": "text"
}
}
}
}
},
"organization": {
"name": "organization",
"fields": {
"name": {
"name": "name",
"type": {
"scalarType": "text"
}
},
"members": {
"name": "committees",
"type": {
"arrayType": {
"compositeType": "committee"
}
}
}
}
}
},
"nativeQueries": {
"summarize_organizations": {
"sql": "SELECT 'The organization ' || org.name || ' has ' || no_committees::text || ' committees, ' || 'the largest of which has ' || max_members || ' members.' AS result FROM (SELECT orgs.* FROM unnest({{organizations}}) as orgs) AS org JOIN LATERAL ( SELECT count(committee.*) AS no_committees, max(members_agg.no_members) AS max_members FROM unnest(org.committees) AS committee JOIN LATERAL ( SELECT count(*) as no_members FROM unnest(committee.members) AS members) AS members_agg ON true) AS coms ON true",
"columns": {
"result": {
"name": "result",
"type": {
"scalarType": "text"
},
"nullable": "nullable",
"description": null
}
},
"arguments": {
"organizations": {
"name": "organizations",
"type": {
"arrayType": {
"compositeType": "organization"
}
},
"nullable": "nullable"
}
},
"description": "A native query used to test support array-valued variables"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
},
"nativeQueries": {
"summarize_organizations": {
"sql": "SELECT 'The organization ' || org.name || ' has ' || no_committees::text || ' committees, ' || 'the largest of which has ' || max_members || ' members.' AS result FROM (SELECT orgs.* FROM unnest({{organizations}})) AS org JOIN LATERAL ( SELECT count(committee.*) AS no_committees, max(members_agg.no_members) AS max_members FROM unnest(org.committees) AS committee JOIN LATERAL ( SELECT count(*) as no_members FROM unnest(committee.members) AS members) AS members_agg ON true) AS coms ON true",
"sql": "SELECT 'The organization ' || org.name || ' has ' || no_committees::text || ' committees, ' || 'the largest of which has ' || max_members || ' members.' AS result FROM (SELECT orgs.* FROM unnest({{organizations}}) AS orgs) AS org JOIN LATERAL ( SELECT count(committee.*) AS no_committees, max(members_agg.no_members) AS max_members FROM unnest(org.committees) AS committee JOIN LATERAL ( SELECT count(*) as no_members FROM unnest(committee.members) AS members) AS members_agg ON true) AS coms ON true",
"columns": {
"result": {
"name": "result",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
---
source: crates/query-engine/translation/tests/tests.rs
expression: result
---
WITH "%1_NATIVE_QUERY_summarize_organizations" AS (
SELECT
'The organization ' || org.name || ' has ' || no_committees :: text || ' committees, ' || 'the largest of which has ' || max_members || ' members.' AS result
FROM
(
SELECT
orgs.*
FROM
unnest(
(
SELECT
array_agg(
jsonb_populate_record(cast(null as organization), "%0_array"."element")
) AS "element"
FROM
jsonb_array_elements($1) AS "%0_array"("element")
)
) as orgs
) AS org
JOIN LATERAL (
SELECT
count(committee.*) AS no_committees,
max(members_agg.no_members) AS max_members
FROM
unnest(org.committees) AS committee
JOIN LATERAL (
SELECT
count(*) as no_members
FROM
unnest(committee.members) AS members
) AS members_agg ON true
) AS coms ON true
)
SELECT
coalesce(json_agg(row_to_json("%2_universe")), '[]') AS "universe"
FROM
(
SELECT
*
FROM
(
SELECT
coalesce(json_agg(row_to_json("%3_rows")), '[]') AS "rows"
FROM
(
SELECT
"%0_summarize_organizations"."result" AS "result"
FROM
"%1_NATIVE_QUERY_summarize_organizations" AS "%0_summarize_organizations"
) AS "%3_rows"
) AS "%3_rows"
) AS "%2_universe"

[(1, Value(Array [Object {"name": String("RC Model Airplane Enthusiasts"), "committees": Array [Object {"name": String("Founders"), "members": Array [Object {"first_name": String("Orville"), "last_name": String("Wright")}, Object {"first_name": String("Wilbur"), "last_name": String("Wright")}]}, Object {"name": String("Parts supply management"), "members": Array [Object {"first_name": String("Orville"), "last_name": String("Wright")}, Object {"first_name": String("Wilbur"), "last_name": String("Wright")}, Object {"first_name": String("Guybrush"), "last_name": String("Threepwood")}]}]}, Object {"name": String("Argonauts' Alumni Association"), "committees": Array [Object {"name": String("Crew"), "members": Array [Object {"first_name": String("Jason"), "last_name": String("(The)")}, Object {"first_name": String("Heracles"), "last_name": String("(The)")}, Object {"first_name": String("Castor"), "last_name": String("(The)")}, Object {"first_name": String("Polydeuces"), "last_name": String("(The)")}]}]}]))]
Loading

0 comments on commit 9eec086

Please sign in to comment.