Skip to content

Commit

Permalink
Use ndc-models directly to avoid openssl everywhere (#156)
Browse files Browse the repository at this point in the history
<!-- The PR description should answer 2 (maybe 3) important questions:
-->

### What

Fix a problem in #155 where
pulling `query-engine-translation` into the configuration crate was
pulling `openssl` in. Turns out we're still pulling `ndc-models` from
the `ndc-sdk` here (probably from before `ndc-models` became it's own
crate).

### How

Use `ndc-models` directly instead of `ndc-sdk`.
  • Loading branch information
danieljharvey authored Dec 11, 2024
1 parent 205ecee commit f529344
Show file tree
Hide file tree
Showing 12 changed files with 162 additions and 166 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/query-engine/translation/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ workspace = true

[dependencies]

ndc-sdk = { workspace = true }
ndc-models = { workspace = true }

query-engine-metadata = { path = "../metadata" }
query-engine-sql = { path = "../sql" }
Expand Down
30 changes: 15 additions & 15 deletions crates/query-engine/translation/src/translation/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use std::collections::BTreeMap;

use ndc_sdk::models::{self, ArgumentName};
use ndc_models::ArgumentName;

use super::error::Error;
use crate::translation::values;
Expand All @@ -13,7 +13,7 @@ use query_engine_sql::sql;
/// Static information from the query and metadata.
pub struct Env<'a> {
metadata: &'a metadata::Metadata,
relationships: BTreeMap<models::RelationshipName, models::Relationship>,
relationships: BTreeMap<ndc_models::RelationshipName, ndc_models::Relationship>,
}

#[derive(Debug)]
Expand Down Expand Up @@ -48,8 +48,8 @@ impl NativeQueries {
#[derive(Debug)]
pub struct MutationOperation {
pub name: String,
pub arguments: BTreeMap<models::ArgumentName, serde_json::Value>,
pub fields: Option<models::NestedField>,
pub arguments: BTreeMap<ndc_models::ArgumentName, serde_json::Value>,
pub fields: Option<ndc_models::NestedField>,
pub kind: MutationOperationKind,
}

Expand All @@ -63,7 +63,7 @@ pub enum MutationOperationKind {
/// Information we store about a native query call.
pub struct NativeQueryInfo {
pub info: metadata::NativeQueryInfo,
pub arguments: BTreeMap<models::ArgumentName, models::Argument>,
pub arguments: BTreeMap<ndc_models::ArgumentName, ndc_models::Argument>,
pub alias: sql::ast::TableAlias,
}

Expand Down Expand Up @@ -152,7 +152,7 @@ pub enum CollectionOrProcedureInfo {
/// a SQL statement that can be run in the DB.
pub fn generate_native_query_sql(
type_arguments: &BTreeMap<ArgumentName, query_engine_metadata::metadata::ColumnInfo>,
native_query_arguments: &BTreeMap<models::ArgumentName, ndc_sdk::models::Argument>,
native_query_arguments: &BTreeMap<ndc_models::ArgumentName, ndc_models::Argument>,
native_query_sql: &NativeQuerySql,
) -> Result<Vec<sql::ast::RawSql>, Error> {
native_query_sql
Expand All @@ -169,10 +169,10 @@ pub fn generate_native_query_sql(
let exp = match native_query_arguments.get(param.as_str()) {
None => Err(Error::ArgumentNotFound(param.to_string())),
Some(argument) => match argument {
models::Argument::Literal { value } => {
ndc_models::Argument::Literal { value } => {
values::translate_json_value(value, &typ)
}
models::Argument::Variable { name } => {
ndc_models::Argument::Variable { name } => {
Ok(values::translate_variable(name, &typ))
}
},
Expand All @@ -187,7 +187,7 @@ impl<'a> Env<'a> {
/// Create a new Env by supplying the metadata and relationships.
pub fn new(
metadata: &'a metadata::Metadata,
relationships: BTreeMap<models::RelationshipName, models::Relationship>,
relationships: BTreeMap<ndc_models::RelationshipName, ndc_models::Relationship>,
) -> Env<'a> {
Env {
metadata,
Expand Down Expand Up @@ -228,7 +228,7 @@ impl<'a> Env<'a> {
/// Lookup a collection's information in the metadata.
pub fn lookup_collection(
&self,
collection_name: &models::CollectionName,
collection_name: &ndc_models::CollectionName,
) -> Result<CollectionOrProcedureInfo, Error> {
let table = self
.metadata
Expand Down Expand Up @@ -282,8 +282,8 @@ impl<'a> Env<'a> {

pub fn lookup_relationship(
&self,
name: &models::RelationshipName,
) -> Result<&models::Relationship, Error> {
name: &ndc_models::RelationshipName,
) -> Result<&ndc_models::Relationship, Error> {
self.relationships
.get(name.as_str())
.ok_or(Error::RelationshipNotFound(name.to_string()))
Expand All @@ -293,7 +293,7 @@ impl<'a> Env<'a> {
pub fn lookup_comparison_operator(
&self,
scalar_type: &metadata::ScalarType,
name: &models::ComparisonOperatorName,
name: &ndc_models::ComparisonOperatorName,
) -> Result<&'a metadata::ComparisonOperator, Error> {
self.metadata
.comparison_operators
Expand All @@ -308,7 +308,7 @@ impl<'a> Env<'a> {
}

impl CollectionOrProcedureInfo {
pub fn lookup_column(&self, column_name: &models::FieldName) -> Result<ColumnInfo, Error> {
pub fn lookup_column(&self, column_name: &ndc_models::FieldName) -> Result<ColumnInfo, Error> {
match &self {
CollectionOrProcedureInfo::Collection(collection_info) => {
collection_info.lookup_column(column_name.as_str())
Expand Down Expand Up @@ -411,7 +411,7 @@ impl State {
&mut self,
name: &str,
info: metadata::NativeQueryInfo,
arguments: BTreeMap<models::ArgumentName, models::Argument>,
arguments: BTreeMap<ndc_models::ArgumentName, ndc_models::Argument>,
) -> sql::ast::TableReference {
let alias = self.make_native_query_table_alias(name);
self.native_queries.native_queries.push(NativeQueryInfo {
Expand Down
67 changes: 33 additions & 34 deletions crates/query-engine/translation/src/translation/mutation/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use indexmap::indexmap;
use indexmap::IndexMap;
use ndc_sdk::models::{self};
use query_engine_metadata::metadata;
use query_engine_sql::sql::execution_plan::{
MutationExecutionPlan, MutationOperationExecutionPlan, NativeMutationOperationExecutionPlan,
Expand Down Expand Up @@ -30,7 +29,7 @@ mod stored_procedures;

pub fn translate(
metadata: &metadata::Metadata,
mutation_request: models::MutationRequest,
mutation_request: ndc_models::MutationRequest,
) -> Result<sql::execution_plan::MutationExecutionPlan, Error> {
let env = Env::new(metadata, mutation_request.collection_relationships);
let state = State::new();
Expand All @@ -52,10 +51,10 @@ pub fn translate(
/// mutation operation.
fn translate_mutation_operation(
env: &Env,
mutation_operation: ndc_sdk::models::MutationOperation,
mutation_operation: ndc_models::MutationOperation,
) -> Result<MutationOperation, Error> {
match mutation_operation {
ndc_sdk::models::MutationOperation::Procedure {
ndc_models::MutationOperation::Procedure {
name,
arguments,
fields,
Expand Down Expand Up @@ -95,62 +94,62 @@ fn translate_mutation_operation(
/// The user must supply at least one of these two structures, and otherwise we'll throw an error.
#[allow(clippy::type_complexity)]
pub fn parse_procedure_fields(
fields: Option<models::NestedField>,
fields: Option<ndc_models::NestedField>,
) -> Result<
(
(
models::FieldName,
Option<IndexMap<models::FieldName, models::Aggregate>>,
ndc_models::FieldName,
Option<IndexMap<ndc_models::FieldName, ndc_models::Aggregate>>,
), // Contains "affected_rows"
(
models::FieldName,
Option<IndexMap<models::FieldName, models::Field>>,
ndc_models::FieldName,
Option<IndexMap<ndc_models::FieldName, ndc_models::Field>>,
), // Contains "returning"
),
Error,
> {
match fields {
Some(models::NestedField::Object(models::NestedObject { fields })) => {
Some(ndc_models::NestedField::Object(ndc_models::NestedObject { fields })) => {
let mut affected_rows = ("affected_rows".into(), None);
let mut returning = ("returning".into(), None);

for (alias, field) in fields {
match field {
models::Field::Column {
ndc_models::Field::Column {
column,
fields: _,
arguments: _,
} if column.as_str() == "affected_rows" => {
affected_rows = (
alias.clone(),
Some(indexmap!(alias => models::Aggregate::StarCount {})),
Some(indexmap!(alias => ndc_models::Aggregate::StarCount {})),
);
}
models::Field::Column {
ndc_models::Field::Column {
column,
fields,
arguments: _,
} if column.as_str() == "returning" => {
returning = match fields {
Some(nested_fields) => match nested_fields {
models::NestedField::Object(models::NestedObject { .. }) => {
Err(Error::UnexpectedStructure(
"single object in 'returning' clause".to_string(),
))?
}
models::NestedField::Array(models::NestedArray { fields }) => {
match &*fields {
models::NestedField::Object(models::NestedObject {
fields,
}) => (alias, Some(fields.clone())),
models::NestedField::Array(_) => {
Err(Error::UnexpectedStructure(
"multi-dimensional array in 'returning' clause"
.to_string(),
))?
}
ndc_models::NestedField::Object(ndc_models::NestedObject {
..
}) => Err(Error::UnexpectedStructure(
"single object in 'returning' clause".to_string(),
))?,
ndc_models::NestedField::Array(ndc_models::NestedArray {
fields,
}) => match &*fields {
ndc_models::NestedField::Object(ndc_models::NestedObject {
fields,
}) => (alias, Some(fields.clone())),
ndc_models::NestedField::Array(_) => {
Err(Error::UnexpectedStructure(
"multi-dimensional array in 'returning' clause"
.to_string(),
))?
}
}
},
},
None => returning,
};
Expand All @@ -168,7 +167,7 @@ pub fn parse_procedure_fields(
Ok((affected_rows, returning))
}

Some(models::NestedField::Array(_)) => {
Some(ndc_models::NestedField::Array(_)) => {
Err(Error::NotImplementedYet("nested array fields".to_string()))
}
None => Err(Error::NoProcedureResultFieldsRequested)?,
Expand Down Expand Up @@ -282,7 +281,7 @@ fn generate_mutation_execution_plan(
.clone()
.into_iter()
.map(|(arg_name, arg_value)| {
(arg_name, models::Argument::Literal { value: arg_value })
(arg_name, ndc_models::Argument::Literal { value: arg_value })
})
.collect(),
&native_mutation_info.info.sql,
Expand All @@ -298,7 +297,7 @@ fn generate_mutation_execution_plan(
let (affected_rows, (returning_alias, returning)) =
parse_procedure_fields(mutation_operation.fields)?;

let query = ndc_sdk::models::Query {
let query = ndc_models::Query {
aggregates: affected_rows.1,
fields: returning,
limit: None,
Expand Down Expand Up @@ -341,7 +340,7 @@ fn generate_mutation_execution_plan(
)?;

// form a single JSON item shaped `{ type: "procedure", result: "<mutation_operation_result>" }`
// that matches the models::RowSet type
// that matches the ndc_models::RowSet type
let json_select = sql::helpers::select_mutation_rowset(
(
state.make_table_alias("universe".to_string()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use crate::translation::{
values::translate_json_value,
};
use indexmap::IndexMap;
use ndc_sdk::models;
use query_engine_metadata::metadata::{ColumnInfo, Nullable};
use query_engine_sql::sql::{
self,
Expand All @@ -20,16 +19,18 @@ use query_engine_sql::sql::{
};

fn parse_stored_procedure_fields(
fields: Option<models::NestedField>,
) -> Result<Option<IndexMap<models::FieldName, models::Field>>, Error> {
fields: Option<ndc_models::NestedField>,
) -> Result<Option<IndexMap<ndc_models::FieldName, ndc_models::Field>>, Error> {
match fields {
None => Ok(None),
Some(models::NestedField::Object(_)) => Err(Error::UnexpectedStructure(
Some(ndc_models::NestedField::Object(_)) => Err(Error::UnexpectedStructure(
"Stored procedures cannot return a single object".to_string(),
)),
Some(models::NestedField::Array(models::NestedArray { fields })) => match *fields {
models::NestedField::Object(models::NestedObject { fields }) => Ok(Some(fields)),
models::NestedField::Array(_) => Err(Error::UnexpectedStructure(
Some(ndc_models::NestedField::Array(ndc_models::NestedArray { fields })) => match *fields {
ndc_models::NestedField::Object(ndc_models::NestedObject { fields }) => {
Ok(Some(fields))
}
ndc_models::NestedField::Array(_) => Err(Error::UnexpectedStructure(
"multi-dimensional array in 'returning' clause".to_string(),
)),
},
Expand All @@ -38,12 +39,12 @@ fn parse_stored_procedure_fields(

fn get_all_procedure_fields(
proc_fields: BTreeMap<String, ColumnInfo>,
) -> IndexMap<models::FieldName, models::Field> {
) -> IndexMap<ndc_models::FieldName, ndc_models::Field> {
let mut fields = IndexMap::new();
for (proc_field_name, proc_field_col_info) in proc_fields {
fields.insert(
proc_field_name.into(),
models::Field::Column {
ndc_models::Field::Column {
arguments: BTreeMap::new(),
column: proc_field_col_info.name.into(),
fields: None,
Expand All @@ -57,8 +58,8 @@ pub(crate) fn generate_execution_plan(
env: &Env,
state: &mut State,
stored_proc_info: StoredProcedureInfo,
requested_fields: Option<ndc_sdk::models::NestedField>,
provided_args: &BTreeMap<models::ArgumentName, serde_json::Value>,
requested_fields: Option<ndc_models::NestedField>,
provided_args: &BTreeMap<ndc_models::ArgumentName, serde_json::Value>,
) -> Result<StoredProcedureExecutionPlan, Error> {
// Compute the fields that need to be returned.
let parsed_fields = parse_stored_procedure_fields(requested_fields)?.unwrap_or(
Expand All @@ -74,7 +75,7 @@ pub(crate) fn generate_execution_plan(
// an `Expression`
for (arg_name, arg_info) in stored_proc_info.info.arguments {
let arg_val: Option<&serde_json::Value> =
provided_args.get::<models::ArgumentName>(&arg_name.clone().into());
provided_args.get::<ndc_models::ArgumentName>(&arg_name.clone().into());

match arg_val {
Some(arg_val) if *arg_val != serde_json::Value::Null => {
Expand Down Expand Up @@ -114,7 +115,7 @@ pub(crate) fn generate_execution_plan(

// Response selection

let query = models::Query {
let query = ndc_models::Query {
aggregates: None,
fields: Some(parsed_fields),
limit: None,
Expand Down Expand Up @@ -151,7 +152,7 @@ pub(crate) fn generate_execution_plan(
)?;

// form a single JSON item shaped `{ type: "procedure", result: "<mutation_operation_result>" }`
// that matches the models::RowSet type
// that matches the ndc_models::RowSet type
let json_select = sql::helpers::select_mutation_rowset(
(
state.make_table_alias("universe".to_string()),
Expand Down
Loading

0 comments on commit f529344

Please sign in to comment.