Skip to content

Commit

Permalink
Introduce v2 of auto-generated mutations (#513)
Browse files Browse the repository at this point in the history
### What

v2 of auto-generated mutations introduces point delete, insert and
update point mutations that include permission arguments.

### Interface and Design

* We generate delete, insert and update procedures for each table.

* A single insert procedure is generated per table of the form:
  ```graphql
  v2_insert_<table>(
      objects: [<object>],
      post_check: <boolexpr>
  )
  ```
It allows us to insert multiple objects and include a post check for
permissions.

* A delete procedure is generated per table X unique constraint of the
form:
  ```graphql
  v2_delete_<table>_by_<column_and_...>(
      key_<column1>: <value>,
      key_<column2>: <value>,
      ...,
      pre_check: <boolexpr>
  )
  ```
It allows us to delete a single row using the uniqueness constraint, and
contains a boolexpr for permissions.

* An update procedure is generated per table X unique constraint of the
form:
  ```graphql
  v2_update_<table>_by_<column_and_...>(
      key_<column1>: <value>,
      key_<column2>: <value>,
      ...,
      update_columns: { <column>: { _set: <value> }, ... },
      pre_check: <boolexpr>,
      post_check: <boolexpr>
  )
  ```
It allows us to update a single row using the uniqueness constraint by
updating the relevant columns,
  and contains a pre check and post check for permissions.

* Mutations using uniqueness constraints use the naming schema
`by_column_and_column_and_column` instead of the db constraint name,
  because the former is far more helpful.
* If generating a mutation encounters an internal error, we skip that
particular mutation and trace a warning instead of throwing
  an error so the connector can start at any situation.
* Naming collisions between the unique constraints and the
update_columns / pre_check / post_check is avoided by prefixing argument
  names of the columns of a unique constraint with `key_`.


### How

- We replace the current `experimental` version of mutations almost word
for word (the only different is the names of the procedures, prefixing
`v2` instead of `experimental`) to a new directory `v2`.
- We add a new mutations version: `v2`.
- We replace the `experimental` version of ndc-spec schema generation as
well.
- We fix the tests to use v2 instead.
- experimental version is also removed from configuration version3
completely.
  • Loading branch information
Gil Mizrahi committed Jun 28, 2024
1 parent c9bf0d4 commit 4bfdad0
Show file tree
Hide file tree
Showing 69 changed files with 2,664 additions and 2,680 deletions.
1 change: 0 additions & 1 deletion crates/configuration/src/version3/metadata/mutations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@ use serde::{Deserialize, Serialize};
#[serde(rename_all = "camelCase")]
pub enum MutationsVersion {
V1,
VeryExperimentalWip,
}
3 changes: 0 additions & 3 deletions crates/configuration/src/version3/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1159,8 +1159,5 @@ fn convert_mutations_version(
metadata::mutations::MutationsVersion::V1 => {
query_engine_metadata::metadata::mutations::MutationsVersion::V1
}
metadata::mutations::MutationsVersion::VeryExperimentalWip => {
query_engine_metadata::metadata::mutations::MutationsVersion::VeryExperimentalWip
}
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -1499,8 +1499,7 @@ expression: schema
"description": "Which version of the generated mutations will be included in the schema",
"type": "string",
"enum": [
"v1",
"veryExperimentalWip"
"v1"
]
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/configuration/src/version4/metadata/mutations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ use serde::{Deserialize, Serialize};
#[serde(rename_all = "camelCase")]
pub enum MutationsVersion {
V1,
VeryExperimentalWip,
V2,
}
Original file line number Diff line number Diff line change
Expand Up @@ -1557,7 +1557,7 @@ expression: schema
"type": "string",
"enum": [
"v1",
"veryExperimentalWip"
"v2"
]
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/configuration/src/version4/to_runtime_configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,8 +497,8 @@ fn convert_mutations_version(
metadata::mutations::MutationsVersion::V1 => {
query_engine_metadata::metadata::mutations::MutationsVersion::V1
}
metadata::mutations::MutationsVersion::VeryExperimentalWip => {
query_engine_metadata::metadata::mutations::MutationsVersion::VeryExperimentalWip
metadata::mutations::MutationsVersion::V2 => {
query_engine_metadata::metadata::mutations::MutationsVersion::V2
}
})
}
3 changes: 0 additions & 3 deletions crates/configuration/src/version4/upgrade_from_v3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,6 @@ fn upgrade_mutations_version(
version3::metadata::mutations::MutationsVersion::V1 => {
metadata::mutations::MutationsVersion::V1
}
version3::metadata::mutations::MutationsVersion::VeryExperimentalWip => {
metadata::mutations::MutationsVersion::VeryExperimentalWip
}
}
}

Expand Down
22 changes: 11 additions & 11 deletions crates/connectors/ndc-postgres/src/schema/mutation/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Generate ndc-spec schema metadata for mutations.

mod experimental;
mod v1;
mod v2;

use std::collections::BTreeMap;

Expand All @@ -24,15 +24,15 @@ pub fn to_procedure(
mutation::generate::Mutation::V1(mutation::v1::Mutation::InsertMutation(insert)) => {
v1::insert_to_procedure(name, insert, object_types, scalar_types)
}
// experimental
mutation::generate::Mutation::Experimental(
mutation::experimental::Mutation::DeleteMutation(delete),
) => experimental::delete_to_procedure(name, delete, object_types, scalar_types),
mutation::generate::Mutation::Experimental(
mutation::experimental::Mutation::InsertMutation(insert),
) => experimental::insert_to_procedure(name, insert, object_types, scalar_types),
mutation::generate::Mutation::Experimental(
mutation::experimental::Mutation::UpdateMutation(update),
) => experimental::update_to_procedure(name, update, object_types, scalar_types),
// v2
mutation::generate::Mutation::V2(mutation::v2::Mutation::DeleteMutation(delete)) => {
v2::delete_to_procedure(name, delete, object_types, scalar_types)
}
mutation::generate::Mutation::V2(mutation::v2::Mutation::InsertMutation(insert)) => {
v2::insert_to_procedure(name, insert, object_types, scalar_types)
}
mutation::generate::Mutation::V2(mutation::v2::Mutation::UpdateMutation(update)) => {
v2::update_to_procedure(name, update, object_types, scalar_types)
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Exposing experimental auto-generated mutations in the ndc-spec Schema.
//! Exposing v2 auto-generated mutations in the ndc-spec Schema.

use std::collections::BTreeMap;

Expand All @@ -8,15 +8,15 @@ use query_engine_translation::translation::mutation;

use super::super::helpers::*;

/// given an experimental `DeleteMutation`, turn it into a `ProcedureInfo` to be output in the schema
/// given an v2 `DeleteMutation`, turn it into a `ProcedureInfo` to be output in the schema
pub fn delete_to_procedure(
name: &String,
delete: &mutation::experimental::delete::DeleteMutation,
delete: &mutation::v2::delete::DeleteMutation,
object_types: &mut BTreeMap<String, models::ObjectType>,
scalar_types: &mut BTreeMap<String, models::ScalarType>,
) -> models::ProcedureInfo {
match delete {
mutation::experimental::delete::DeleteMutation::DeleteByKey {
mutation::v2::delete::DeleteMutation::DeleteByKey {
by_columns,
pre_check,
description,
Expand Down Expand Up @@ -61,14 +61,14 @@ pub fn delete_to_procedure(
}
}

/// Given an experimental `UpdateMutation`, turn it into a `ProcedureInfo` to be output in the schema.
/// Given an v2 `UpdateMutation`, turn it into a `ProcedureInfo` to be output in the schema.
pub fn update_to_procedure(
procedure_name: &str,
update: &mutation::experimental::update::UpdateMutation,
update: &mutation::v2::update::UpdateMutation,
object_types: &mut BTreeMap<String, models::ObjectType>,
scalar_types: &mut BTreeMap<String, models::ScalarType>,
) -> models::ProcedureInfo {
let mutation::experimental::update::UpdateMutation::UpdateByKey(update_by_key) = update;
let mutation::v2::update::UpdateMutation::UpdateByKey(update_by_key) = update;

let mut arguments = BTreeMap::new();

Expand Down Expand Up @@ -179,10 +179,10 @@ pub fn update_to_procedure(
)
}

/// Given an experimental `InsertMutation`, turn it into a `ProcedureInfo` to be output in the schema.
/// Given an v2 `InsertMutation`, turn it into a `ProcedureInfo` to be output in the schema.
pub fn insert_to_procedure(
name: &String,
insert: &mutation::experimental::insert::InsertMutation,
insert: &mutation::v2::insert::InsertMutation,
object_types: &mut BTreeMap<String, models::ObjectType>,
scalar_types: &mut BTreeMap<String, models::ScalarType>,
) -> models::ProcedureInfo {
Expand Down
2 changes: 1 addition & 1 deletion crates/query-engine/metadata/src/metadata/mutations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
pub enum MutationsVersion {
V1,
VeryExperimentalWip,
V2,
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ use std::collections::BTreeMap;

use crate::translation::helpers::Env;

use super::experimental;
use super::v1;
use super::v2;

#[derive(Debug, Clone)]
pub enum Mutation {
V1(v1::Mutation),
Experimental(experimental::Mutation),
V2(v2::Mutation),
}

/// Given our introspection data, work out all the mutations we can generate
Expand All @@ -21,12 +21,10 @@ pub fn generate(env: &Env) -> BTreeMap<String, Mutation> {
.into_iter()
.map(|(name, mutation)| (name, Mutation::V1(mutation)))
.collect(),
Some(mutations::MutationsVersion::VeryExperimentalWip) => {
experimental::generate(&env.metadata.tables)
.into_iter()
.map(|(name, mutation)| (name, Mutation::Experimental(mutation)))
.collect()
}
Some(mutations::MutationsVersion::V2) => v2::generate(&env.metadata.tables)
.into_iter()
.map(|(name, mutation)| (name, Mutation::V2(mutation)))
.collect(),
None => BTreeMap::new(),
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
pub mod check_columns;
pub mod experimental;
pub mod generate;
pub mod translate;
pub mod v1;
pub mod v2;

pub use translate::translate;
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use crate::translation::helpers::{Env, State};
use query_engine_metadata::metadata;
use query_engine_sql::sql;

use super::experimental;
use super::v1;
use super::v2;

/// Translate the incoming MutationOperation to an ExecutionPlan (SQL) to be run against the database.
pub fn translate(
Expand Down Expand Up @@ -341,8 +341,8 @@ fn translate_mutation_expr(
Some(metadata::mutations::MutationsVersion::V1) => {
v1::translate(env, state, procedure_name, arguments)
}
Some(metadata::mutations::MutationsVersion::VeryExperimentalWip) => {
experimental::translate(env, state, procedure_name, arguments)
Some(metadata::mutations::MutationsVersion::V2) => {
v2::translate(env, state, procedure_name, arguments)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ pub fn generate_delete_by_unique(
keys,
)?;

let name = format!("experimental_delete_{collection_name}_by_{constraint_name}",);
let name = format!(
"{}_delete_{collection_name}_by_{constraint_name}",
super::VERSION
);

let description = format!(
"Delete any row on the '{collection_name}' collection using the {}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub fn generate(
collection_name: &str,
table_info: &database::TableInfo,
) -> (String, InsertMutation) {
let name = format!("experimental_insert_{collection_name}");
let name = format!("{}_insert_{collection_name}", super::VERSION);

let description = format!("Insert into the {collection_name} table");

Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
//! Experimental mutations version is a work in progress version that is expected to be renamed once ready.
//! This version in particular is supposed to introduce permission predicates via boolean expressions.
//! V2 mutations version.
//! This version in particular introduced permission predicates via boolean expressions and update mutations.
//!
//! Design and decisions:
//!
//! * We generate delete, insert and update procedures for each table.
//!
//! * A single insert procedure is generated per table of the form:
//! > <version>_insert_<table>(
//! > v2_insert_<table>(
//! > objects: [<object>],
//! > post_check: <boolexpr>
//! > )
//! It allows us to insert multiple objects and include a post check for permissions.
//!
//! * A delete procedure is generated per table X unique constraint of the form:
//! > <version>_delete_<table>_by_<column_and_...>(
//! > v2_delete_<table>_by_<column_and_...>(
//! > key_<column1>: <value>,
//! > key_<column2>: <value>,
//! > ...,
Expand All @@ -22,7 +22,7 @@
//! It allows us to delete a single row using the uniqueness constraint, and contains a boolexpr for permissions.
//!
//! * An update procedure is generated per table X unique constraint of the form:
//! > <version>_update_<table>_by_<column_and_...>(
//! > v2_update_<table>_by_<column_and_...>(
//! > key_<column1>: <value>,
//! > key_<column2>: <value>,
//! > ...,
Expand All @@ -49,3 +49,5 @@ pub mod update;

pub use generate::{generate, Mutation};
pub use translate::translate;

pub static VERSION: &str = "v2";
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ pub fn generate_update_by_unique(
keys,
)?;

let name = format!("experimental_update_{collection_name}_by_{constraint_name}",);
let name = format!(
"{}_update_{collection_name}_by_{constraint_name}",
super::VERSION
);

let description = format!(
"Update any row on the '{collection_name}' collection using the {}",
Expand Down
2 changes: 1 addition & 1 deletion crates/query-engine/translation/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ pub async fn test_mutation_translation(
&metadata,
operation,
request.collection_relationships.clone(),
Some(query_engine_metadata::metadata::mutations::MutationsVersion::VeryExperimentalWip),
Some(query_engine_metadata::metadata::mutations::MutationsVersion::V2),
)
})
.collect::<Result<Vec<_>, translation::error::Error>>()?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,5 @@
"compositeTypes": {},
"nativeQueries": {}
},
"mutationsVersion": "veryExperimentalWip"
"mutationsVersion": "v2"
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"operations": [
{
"type": "procedure",
"name": "experimental_insert_Artist",
"name": "v2_insert_Artist",
"arguments": {
"objects": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,5 @@
"compositeTypes": {},
"nativeQueries": {}
},
"mutationsVersion": "veryExperimentalWip"
"mutationsVersion": "v2"
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"operations": [
{
"type": "procedure",
"name": "experimental_insert_Dog",
"name": "v2_insert_Dog",
"arguments": {
"objects": [{}, {}, {}, {}],
"post_check": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,11 @@ SELECT
(
SELECT
coalesce(
bool_and(
"%7_experimental_insert_Artist"."%check__constraint"
),
bool_and("%7_v2_insert_Artist"."%check__constraint"),
true
) AS "%check__constraint"
FROM
"%0_generated_mutation" AS "%7_experimental_insert_Artist"
"%0_generated_mutation" AS "%7_v2_insert_Artist"
) AS "%check__constraint";

COMMIT;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,11 @@ SELECT
(
SELECT
coalesce(
bool_and(
"%7_experimental_insert_Dog"."%check__constraint"
),
bool_and("%7_v2_insert_Dog"."%check__constraint"),
true
) AS "%check__constraint"
FROM
"%0_generated_mutation" AS "%7_experimental_insert_Dog"
"%0_generated_mutation" AS "%7_v2_insert_Dog"
) AS "%check__constraint";

COMMIT;
Expand Down
21 changes: 9 additions & 12 deletions crates/query-engine/translation/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,22 +407,19 @@ mod mutations {
}

#[tokio::test]
async fn experimental_insert() {
let result =
common::test_mutation_translation(IsolationLevel::default(), "experimental_insert")
.await
.unwrap();
async fn v2_insert() {
let result = common::test_mutation_translation(IsolationLevel::default(), "v2_insert")
.await
.unwrap();
insta::assert_snapshot!(result);
}

#[tokio::test]
async fn experimental_insert_empty_objects() {
let result = common::test_mutation_translation(
IsolationLevel::default(),
"experimental_insert_empty_objects",
)
.await
.unwrap();
async fn v2_insert_empty_objects() {
let result =
common::test_mutation_translation(IsolationLevel::default(), "v2_insert_empty_objects")
.await
.unwrap();
insta::assert_snapshot!(result);
}
}
Loading

0 comments on commit 4bfdad0

Please sign in to comment.