Skip to content
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

Make mutation pre/post checks optional #635

Merged
merged 5 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

### Changed

- Change mutation pre/post checks to be optional

### Fixed

## [v1.1.2] - 2024-10-07
Expand Down
95 changes: 47 additions & 48 deletions crates/connectors/ndc-postgres/src/schema/mutation/v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,50 +16,42 @@ pub fn delete_to_procedure(
object_types: &mut BTreeMap<models::ObjectTypeName, models::ObjectType>,
scalar_types: &mut BTreeMap<models::ScalarTypeName, models::ScalarType>,
) -> models::ProcedureInfo {
match delete {
mutation::v2::delete::DeleteMutation::DeleteByKey {
by_columns,
pre_check,
description,
collection_name,
columns_prefix,
table_name: _,
schema_name: _,
} => {
let mut arguments = BTreeMap::new();

for column in by_columns {
arguments.insert(
format!("{}{}", columns_prefix, column.name).into(),
models::ArgumentInfo {
argument_type: column_to_type(column),
description: column.description.clone(),
},
);
}

arguments.insert(
pre_check.argument_name.clone(),
models::ArgumentInfo {
argument_type: models::Type::Predicate {
object_type_name: collection_name.as_str().into(),
},
description: Some(pre_check.description.clone()),
},
);
let mutation::v2::delete::DeleteMutation::DeleteByKey(delete_by_key) = delete;

make_procedure_type(
name.clone(),
Some(description.to_string()),
arguments,
models::Type::Named {
name: collection_name.as_str().into(),
},
object_types,
scalar_types,
)
}
let mut arguments = BTreeMap::new();

for column in &delete_by_key.by_columns {
arguments.insert(
format!("{}{}", delete_by_key.columns_prefix, column.name).into(),
models::ArgumentInfo {
argument_type: column_to_type(column),
description: column.description.clone(),
},
);
}

arguments.insert(
delete_by_key.pre_check.argument_name.clone(),
models::ArgumentInfo {
argument_type: models::Type::Nullable {
underlying_type: Box::new(models::Type::Predicate {
object_type_name: delete_by_key.collection_name.as_str().into(),
}),
},
description: Some(delete_by_key.pre_check.description.clone()),
},
);

make_procedure_type(
name.clone(),
Some(delete_by_key.description.to_string()),
arguments,
models::Type::Named {
name: delete_by_key.collection_name.as_str().into(),
},
object_types,
scalar_types,
)
}

/// Given an v2 `UpdateMutation`, turn it into a `ProcedureInfo` to be output in the schema.
Expand Down Expand Up @@ -88,8 +80,10 @@ pub fn update_to_procedure(
arguments.insert(
update_by_key.pre_check.argument_name.clone(),
models::ArgumentInfo {
argument_type: models::Type::Predicate {
object_type_name: update_by_key.collection_name.as_str().into(),
argument_type: models::Type::Nullable {
underlying_type: Box::new(models::Type::Predicate {
object_type_name: update_by_key.collection_name.as_str().into(),
}),
},
description: Some(update_by_key.pre_check.description.clone()),
},
Expand All @@ -99,8 +93,10 @@ pub fn update_to_procedure(
arguments.insert(
update_by_key.post_check.argument_name.clone(),
models::ArgumentInfo {
argument_type: models::Type::Predicate {
object_type_name: update_by_key.collection_name.as_str().into(),
argument_type: models::Type::Nullable {
underlying_type: Box::new(models::Type::Predicate {
object_type_name: update_by_key.collection_name.as_str().into(),
}),
},
description: Some(update_by_key.post_check.description.clone()),
},
Expand Down Expand Up @@ -209,11 +205,14 @@ pub fn insert_to_procedure(
description: None,
},
);

arguments.insert(
insert.post_check.argument_name.clone(),
models::ArgumentInfo {
argument_type: models::Type::Predicate {
object_type_name: insert.collection_name.as_str().into(),
argument_type: models::Type::Nullable {
underlying_type: Box::new(models::Type::Predicate {
object_type_name: insert.collection_name.as_str().into(),
}),
},
description: Some(insert.post_check.description.clone()),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,8 @@ pub struct CheckArgument {
pub argument_name: models::ArgumentName,
pub description: String,
}

// Default check argument/constraint
pub fn default_constraint() -> serde_json::Value {
serde_json::json!({"type": "and", "expressions": []})
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,26 @@ use query_engine_metadata::metadata::database;
use query_engine_sql::sql;
use std::collections::BTreeMap;

use super::common::{self, CheckArgument};
use super::common::{self, default_constraint, CheckArgument};

/// A representation of an auto-generated delete mutation.
///
/// This can get us `DELETE FROM <table> WHERE column = <column_name_arg>, ...`.
#[derive(Debug, Clone)]
pub enum DeleteMutation {
DeleteByKey {
description: String,
collection_name: models::CollectionName,
schema_name: sql::ast::SchemaName,
table_name: sql::ast::TableName,
by_columns: NonEmpty<metadata::database::ColumnInfo>,
columns_prefix: String,
pre_check: CheckArgument,
},
DeleteByKey(DeleteByKey),
}

/// A representation of an auto-generated delete mutation by a unique key.
#[derive(Debug, Clone)]
pub struct DeleteByKey {
pub description: String,
pub collection_name: models::CollectionName,
pub schema_name: sql::ast::SchemaName,
pub table_name: sql::ast::TableName,
pub by_columns: NonEmpty<metadata::database::ColumnInfo>,
pub columns_prefix: String,
pub pre_check: CheckArgument,
}

/// generate a delete for each simple unique constraint on this table
Expand Down Expand Up @@ -59,7 +63,7 @@ pub fn generate_delete_by_unique(
common::description_keys(&keys.0.values().collect())
);

let delete_mutation = DeleteMutation::DeleteByKey {
let delete_mutation = DeleteMutation::DeleteByKey(DeleteByKey {
schema_name: sql::ast::SchemaName(table_info.schema_name.clone()),
table_name: sql::ast::TableName(table_info.table_name.clone()),
collection_name: collection_name.clone(),
Expand All @@ -72,7 +76,7 @@ pub fn generate_delete_by_unique(
),
},
description,
};
});

Some((name, delete_mutation))
})
Expand All @@ -83,29 +87,21 @@ pub fn generate_delete_by_unique(
pub fn translate(
env: &crate::translation::helpers::Env,
state: &mut crate::translation::helpers::State,
delete: &DeleteMutation,
mutation: &DeleteMutation,
arguments: &BTreeMap<models::ArgumentName, serde_json::Value>,
) -> Result<(sql::ast::Delete, sql::ast::ColumnAlias), Error> {
match delete {
DeleteMutation::DeleteByKey {
collection_name,
schema_name,
table_name,
by_columns,
columns_prefix,
pre_check,
description: _,
} => {
match mutation {
DeleteMutation::DeleteByKey(mutation) => {
// The root table we are going to be deleting from.
let table = sql::ast::TableReference::DBTable {
schema: schema_name.clone(),
table: table_name.clone(),
schema: mutation.schema_name.clone(),
table: mutation.table_name.clone(),
};

let table_alias = state.make_table_alias(table_name.0.clone());
let table_alias = state.make_table_alias(mutation.table_name.0.clone());

let table_name_and_reference = TableSourceAndReference {
source: helpers::TableSource::Collection(collection_name.clone()),
source: helpers::TableSource::Collection(mutation.collection_name.clone()),
reference: sql::ast::TableReference::AliasedTable(table_alias.clone()),
};

Expand All @@ -115,10 +111,12 @@ pub fn translate(
};

// Build the `UNIQUE_KEY = <value>, ...` boolean expression.
let unique_expressions = by_columns
let unique_expressions = mutation
.by_columns
.iter()
.map(|by_column| {
let argument_name = format!("{}{}", columns_prefix, by_column.name).into();
let argument_name =
format!("{}{}", mutation.columns_prefix, by_column.name).into();
let unique_key = arguments
.get(&argument_name)
.ok_or(Error::ArgumentNotFound(argument_name))?;
Expand All @@ -141,15 +139,16 @@ pub fn translate(
.collect::<Result<Vec<sql::ast::Expression>, Error>>()?;

// Build the `pre_check` argument boolean expression.
let default_constraint = default_constraint();
let predicate_json = arguments
.get(&pre_check.argument_name)
.ok_or(Error::ArgumentNotFound(pre_check.argument_name.clone()))?;
.get(&mutation.pre_check.argument_name)
.unwrap_or(&default_constraint);

let predicate: models::Expression = serde_json::from_value(predicate_json.clone())
.map_err(|_| {
Error::UnexpectedStructure(format!(
"Argument '{}' should have an ndc-spec Expression structure",
pre_check.argument_name.clone()
mutation.pre_check.argument_name.clone()
))
})?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use query_engine_metadata::metadata::database;
use query_engine_sql::sql;
use std::collections::{BTreeMap, BTreeSet};

use super::common::CheckArgument;
use super::common::{default_constraint, CheckArgument};

/// A representation of an auto-generated insert mutation.
///
Expand Down Expand Up @@ -224,12 +224,10 @@ pub fn translate(
};

// Build the `post_check` argument boolean expression.
let predicate_json =
arguments
.get(&mutation.post_check.argument_name)
.ok_or(Error::ArgumentNotFound(
mutation.post_check.argument_name.clone(),
))?;
let default_constraint = default_constraint();
let predicate_json = arguments
.get(&mutation.post_check.argument_name)
.unwrap_or(&default_constraint);

let predicate: models::Expression =
serde_json::from_value(predicate_json.clone()).map_err(|_| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use crate::translation::helpers::{Env, State};
use ndc_models as models;
use query_engine_sql::sql;

use super::delete::DeleteByKey;

/// Translate a built-in delete mutation into an ExecutionPlan (SQL) to be run against the database.
/// This part is specialized for this mutations versions.
/// To be invoke from the main mutations translate function.
Expand All @@ -28,10 +30,10 @@ pub fn translate(
Ok(match mutation {
super::generate::Mutation::DeleteMutation(delete) => {
let return_collection = match delete {
super::delete::DeleteMutation::DeleteByKey {
super::delete::DeleteMutation::DeleteByKey(DeleteByKey {
ref collection_name,
..
} => collection_name.clone(),
}) => collection_name.clone(),
};

let (delete_cte, check_constraint_alias) =
Expand Down
Loading