Make v2_ mutation prefix configurable (#636)

<!-- The PR description should answer 2 (maybe 3) important questions:
-->

### What

Pulled from #635 into own PR.

We add `mutations_prefix: Option<String>` to v5 configuration. When left
`null` (for existing projects), we default to `v2_mutation_name`, but it
can be replaced with `horse_mutation_name` etc.

Most people will just want to use it to remove the prefix though, and
that is the default for new projects. `mutationsPrefix: ""` will get rid
of the prefix altogether, which I assume most users would want.

### How

<!-- How is it trying to accomplish it (what are the implementation
steps)? -->

---------

Co-authored-by: Brandon Martin <brandon@codedmart.com>

changelog.md

@@ -8,6 +8,9 @@
 
 - Change mutation pre/post checks to be optional
 
+- Added `mutationPrefix` to configuration allowing mutation names to be
+  customised.
+
 ### Fixed
 
 ## [v1.1.2] - 2024-10-07

crates/configuration/src/configuration.rs

@@ -75,6 +75,7 @@ pub struct Configuration {
     pub connection_uri: String,
     pub isolation_level: IsolationLevel,
     pub mutations_version: Option<metadata::mutations::MutationsVersion>,
+    pub mutations_prefix: Option<String>,
 }
 pub async fn introspect(
     input: ParsedConfiguration,

crates/configuration/src/version3/mod.rs

@@ -512,6 +512,7 @@ pub fn make_runtime_configuration(
         isolation_level: configuration.connection_settings.isolation_level,
         mutations_version: convert_mutations_version(configuration.mutations_version),
         configuration_version_tag: VersionTag::Version3,
+        mutations_prefix: None,
     })
 }
 

crates/configuration/src/version4/to_runtime_configuration.rs

@@ -34,6 +34,7 @@ pub fn make_runtime_configuration(
         isolation_level: parsed_config.connection_settings.isolation_level,
         mutations_version: convert_mutations_version(parsed_config.mutations_version),
         configuration_version_tag: VersionTag::Version4,
+        mutations_prefix: None,
     })
 }
 

crates/configuration/src/version5/mod.rs

@@ -51,6 +51,9 @@ pub struct ParsedConfiguration {
     /// Which version of the generated mutation procedures to include in the schema response
     #[serde(default)]
     pub mutations_version: Option<metadata::mutations::MutationsVersion>,
+    /// Provide a custom prefix for generated mutation names. Defaults to mutations version.
+    #[serde(default)]
+    pub mutations_prefix: Option<String>,
 }
 
 #[derive(Clone, PartialEq, Eq, Debug, Deserialize, Serialize, JsonSchema)]
@@ -68,6 +71,7 @@ impl ParsedConfiguration {
             metadata: metadata::Metadata::default(),
             introspection_options: options::IntrospectionOptions::default(),
             mutations_version: Some(metadata::mutations::MutationsVersion::V2),
+            mutations_prefix: Some(String::new()),
         }
     }
 
@@ -192,6 +196,7 @@ pub async fn introspect(
         },
         introspection_options: args.introspection_options,
         mutations_version: args.mutations_version,
+        mutations_prefix: args.mutations_prefix,
     })
 }
 

crates/configuration/src/version5/to_runtime_configuration.rs

@@ -34,6 +34,7 @@ pub fn make_runtime_configuration(
         isolation_level: parsed_config.connection_settings.isolation_level,
         mutations_version: convert_mutations_version(parsed_config.mutations_version),
         configuration_version_tag: VersionTag::Version4,
+        mutations_prefix: parsed_config.mutations_prefix,
     })
 }
 

crates/configuration/src/version5/upgrade_from_v4.rs

@@ -26,6 +26,7 @@ pub fn upgrade_from_v4(v: version4::ParsedConfiguration) -> super::ParsedConfigu
         introspection_options: ugrade_introspection_options(introspection_options),
         metadata: upgrade_metadata(metadata),
         mutations_version: mutations_version.map(upgrade_mutations_version),
+        mutations_prefix: Some(String::new()), // default to no prefixes
     }
 }
 

crates/connectors/ndc-postgres/src/mutation.rs

@@ -85,6 +85,7 @@ fn plan_mutation(
                 operation,
                 request.collection_relationships.clone(),
                 configuration.mutations_version,
+                configuration.mutations_prefix.clone(),
             )
         })
         .collect::<Result<Vec<_>, _>>()?;

crates/connectors/ndc-postgres/src/schema/mod.rs

@@ -333,7 +333,13 @@ pub fn get_schema(
         .collect();
 
     let mut more_object_types = BTreeMap::new();
-    let env = Env::new(metadata, BTreeMap::new(), config.mutations_version, None);
+    let env = Env::new(
+        metadata,
+        BTreeMap::new(),
+        config.mutations_version,
+        config.mutations_prefix.clone(),
+        None,
+    );
     let generated_procedures: Vec<models::ProcedureInfo> =
         query_engine_translation::translation::mutation::generate::generate(&env)
             .iter()

crates/query-engine/translation/src/translation/helpers.rs

@@ -14,6 +14,7 @@ pub struct Env<'request> {
     pub(crate) metadata: &'request metadata::Metadata,
     relationships: BTreeMap<models::RelationshipName, models::Relationship>,
     pub(crate) mutations_version: Option<metadata::mutations::MutationsVersion>,
+    pub(crate) mutations_prefix: Option<String>,
     variables_table: Option<sql::ast::TableReference>,
 }
 
@@ -208,6 +209,7 @@ impl<'request> Env<'request> {
             metadata: &temp_metadata,
             relationships: BTreeMap::new(),
             mutations_version: None,
+            mutations_prefix: None,
             variables_table: None,
         };
         f(temp_env)
@@ -218,12 +220,14 @@ impl<'request> Env<'request> {
         metadata: &'request metadata::Metadata,
         relationships: BTreeMap<models::RelationshipName, models::Relationship>,
         mutations_version: Option<metadata::mutations::MutationsVersion>,
+        mutations_prefix: Option<String>,
         variables_table: Option<sql::ast::TableReference>,
     ) -> Self {
         Env {
             metadata,
             relationships,
             mutations_version,
+            mutations_prefix,
             variables_table,
         }
     }

crates/query-engine/translation/src/translation/mutation/generate.rs

@@ -22,10 +22,12 @@ pub fn generate(env: &Env) -> BTreeMap<models::ProcedureName, Mutation> {
             .into_iter()
             .map(|(name, mutation)| (name, Mutation::V1(mutation)))
             .collect(),
-        Some(mutations::MutationsVersion::V2) => v2::generate(&env.metadata.tables)
-            .into_iter()
-            .map(|(name, mutation)| (name, Mutation::V2(mutation)))
-            .collect(),
+        Some(mutations::MutationsVersion::V2) => {
+            v2::generate(&env.metadata.tables, &env.mutations_prefix)
+                .into_iter()
+                .map(|(name, mutation)| (name, Mutation::V2(mutation)))
+                .collect()
+        }
         None => BTreeMap::new(),
     }
 }

crates/query-engine/translation/src/translation/mutation/translate.rs

@@ -19,8 +19,15 @@ pub fn translate(
     operation: models::MutationOperation,
     collection_relationships: BTreeMap<models::RelationshipName, models::Relationship>,
     mutations_version: Option<metadata::mutations::MutationsVersion>,
+    mutations_prefix: Option<String>,
 ) -> Result<sql::execution_plan::Mutation, Error> {
-    let env = Env::new(metadata, collection_relationships, mutations_version, None);
+    let env = Env::new(
+        metadata,
+        collection_relationships,
+        mutations_version,
+        mutations_prefix,
+        None,
+    );
 
     match operation {
         models::MutationOperation::Procedure {

crates/query-engine/translation/src/translation/mutation/v2/common.rs

@@ -107,3 +107,25 @@ pub struct CheckArgument {
 pub fn default_constraint() -> serde_json::Value {
     serde_json::json!({"type": "and", "expressions": []})
 }
+
+// the old default was to prefix generated mutations with `v2_` or `v1_`
+// but now we are able to override this
+pub fn get_version_prefix(mutations_prefix: &Option<String>) -> String {
+    match mutations_prefix {
+        None => format!("{}_", super::VERSION),
+        Some(str) => match str.as_str() {
+            "" => String::new(),
+            _ => format!("{str}_"),
+        },
+    }
+}
+
+#[test]
+fn test_version_prefix() {
+    assert_eq!(get_version_prefix(&None), "v2_".to_string());
+    assert_eq!(
+        get_version_prefix(&Some("horse".into())),
+        "horse_".to_string()
+    );
+    assert_eq!(get_version_prefix(&Some(String::new())), String::new());
+}

crates/query-engine/translation/src/translation/mutation/v2/delete.rs

@@ -37,6 +37,7 @@ pub struct DeleteByKey {
 pub fn generate_delete_by_unique(
     collection_name: &models::CollectionName,
     table_info: &database::TableInfo,
+    mutations_prefix: &Option<String>,
 ) -> Vec<(models::ProcedureName, DeleteMutation)> {
     table_info
         .uniqueness_constraints
@@ -53,8 +54,8 @@ pub fn generate_delete_by_unique(
                 )?;
 
             let name = format!(
-                "{}_delete_{collection_name}_by_{constraint_name}",
-                super::VERSION
+                "{}delete_{collection_name}_by_{constraint_name}",
+                common::get_version_prefix(mutations_prefix)
             )
             .into();
 

crates/query-engine/translation/src/translation/mutation/v2/generate.rs

@@ -16,21 +16,27 @@ pub enum Mutation {
 }
 
 /// Given our introspection data, work out all the mutations we can generate
-pub fn generate(tables_info: &database::TablesInfo) -> BTreeMap<models::ProcedureName, Mutation> {
+pub fn generate(
+    tables_info: &database::TablesInfo,
+    mutations_prefix: &Option<String>,
+) -> BTreeMap<models::ProcedureName, Mutation> {
     let mut mutations = BTreeMap::new();
     for (collection_name, table_info) in &tables_info.0 {
         // Delete mutations.
-        let delete_mutations = generate_delete_by_unique(collection_name, table_info);
+        let delete_mutations =
+            generate_delete_by_unique(collection_name, table_info, mutations_prefix);
         for (name, delete_mutation) in delete_mutations {
             mutations.insert(name, Mutation::DeleteMutation(delete_mutation));
         }
 
         // Insert mutations.
-        let (name, insert_mutation) = insert::generate(collection_name, table_info);
+        let (name, insert_mutation) =
+            insert::generate(collection_name, table_info, mutations_prefix);
         mutations.insert(name, Mutation::InsertMutation(insert_mutation));
 
         // Update mutations.
-        let update_mutations = generate_update_by_unique(collection_name, table_info);
+        let update_mutations =
+            generate_update_by_unique(collection_name, table_info, mutations_prefix);
         for (name, update_mutation) in update_mutations {
             mutations.insert(name, Mutation::UpdateMutation(update_mutation));
         }

crates/query-engine/translation/src/translation/mutation/v2/insert.rs

@@ -11,7 +11,7 @@ use query_engine_metadata::metadata::database;
 use query_engine_sql::sql;
 use std::collections::{BTreeMap, BTreeSet};
 
-use super::common::{default_constraint, CheckArgument};
+use super::common::{self, default_constraint, CheckArgument};
 
 /// A representation of an auto-generated insert mutation.
 ///
@@ -31,8 +31,13 @@ pub struct InsertMutation {
 pub fn generate(
     collection_name: &models::CollectionName,
     table_info: &database::TableInfo,
+    mutations_prefix: &Option<String>,
 ) -> (models::ProcedureName, InsertMutation) {
-    let name = format!("{}_insert_{collection_name}", super::VERSION).into();
+    let name = format!(
+        "{}insert_{collection_name}",
+        common::get_version_prefix(mutations_prefix)
+    )
+    .into();
 
     let description = format!("Insert into the {collection_name} table");
 

crates/query-engine/translation/src/translation/mutation/v2/mod.rs

@@ -7,7 +7,7 @@
 //!
 //! * A single insert procedure is generated per table of the form:
 //!
-//!   > v2_insert_<table>(
+//!   > insert_<table>(
 //!   >     objects: [<object>],
 //!   >     post_check: <boolexpr>
 //!   > )

crates/query-engine/translation/src/translation/mutation/v2/translate.rs

@@ -25,7 +25,7 @@ pub fn translate(
     ),
     Error,
 > {
-    let mutation = lookup_generated_mutation(env, procedure_name)?;
+    let mutation = lookup_generated_mutation(env, procedure_name, &env.mutations_prefix)?;
 
     Ok(match mutation {
         super::generate::Mutation::DeleteMutation(delete) => {
@@ -79,11 +79,12 @@ pub fn translate(
 fn lookup_generated_mutation(
     env: &Env<'_>,
     procedure_name: &models::ProcedureName,
+    mutations_prefix: &Option<String>,
 ) -> Result<super::generate::Mutation, Error> {
     // this means we generate them on every mutation request
     // i don't think this is optimal but I'd like to get this working before working out
     // where best to store these
-    let generated = super::generate::generate(&env.metadata.tables);
+    let generated = super::generate::generate(&env.metadata.tables, mutations_prefix);
 
     generated
         .get(procedure_name)

crates/query-engine/translation/src/translation/mutation/v2/update.rs

@@ -41,6 +41,7 @@ pub struct UpdateByKey {
 pub fn generate_update_by_unique(
     collection_name: &models::CollectionName,
     table_info: &database::TableInfo,
+    mutations_prefix: &Option<String>,
 ) -> Vec<(models::ProcedureName, UpdateMutation)> {
     table_info
         .uniqueness_constraints
@@ -57,8 +58,8 @@ pub fn generate_update_by_unique(
                 )?;
 
             let name = format!(
-                "{}_update_{collection_name}_by_{constraint_name}",
-                super::VERSION
+                "{}update_{collection_name}_by_{constraint_name}",
+                common::get_version_prefix(mutations_prefix)
             )
             .into();
 

crates/query-engine/translation/src/translation/query/mod.rs

@@ -29,6 +29,7 @@ pub fn translate(
         metadata,
         query_request.collection_relationships,
         None,
+        None,
         variables_table_ref,
     );
 

crates/query-engine/translation/tests/common/mod.rs

@@ -94,6 +94,7 @@ pub async fn test_mutation_translation(
                 operation,
                 request.collection_relationships.clone(),
                 Some(query_engine_metadata::metadata::mutations::MutationsVersion::V2),
+                configuration.mutations_prefix.clone(),
             )
         })
         .collect::<Result<Vec<_>, translation::error::Error>>()?;

crates/tests/databases-tests/src/postgres/explain_tests.rs

@@ -163,12 +163,12 @@ mod mutation {
             &["Insert", "Aggregate"],
             result
                 .details
-                .get("0 v2_insert_custom_dog Execution Plan")
+                .get("0 insert_custom_dog Execution Plan")
                 .unwrap(),
         );
         insta::assert_snapshot!(result
             .details
-            .get("0 v2_insert_custom_dog SQL Mutation")
+            .get("0 insert_custom_dog SQL Mutation")
             .unwrap());
     }
 
@@ -185,12 +185,12 @@ mod mutation {
             ],
             result
                 .details
-                .get("0 v2_delete_InvoiceLine_by_InvoiceLineId Execution Plan")
+                .get("0 delete_InvoiceLine_by_InvoiceLineId Execution Plan")
                 .unwrap(),
         );
         insta::assert_snapshot!(result
             .details
-            .get("0 v2_delete_InvoiceLine_by_InvoiceLineId SQL Mutation")
+            .get("0 delete_InvoiceLine_by_InvoiceLineId SQL Mutation")
             .unwrap());
     }
 
@@ -202,12 +202,12 @@ mod mutation {
             &["Update", "Aggregate"],
             result
                 .details
-                .get("1 v2_update_custom_dog_by_id Execution Plan")
+                .get("1 update_custom_dog_by_id Execution Plan")
                 .unwrap(),
         );
         insta::assert_snapshot!(result
             .details
-            .get("1 v2_update_custom_dog_by_id SQL Mutation")
+            .get("1 update_custom_dog_by_id SQL Mutation")
             .unwrap());
     }
 }

crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__cli_version5_tests__postgres_current_only_configure_initial_configuration_is_unchanged.snap

@@ -3123,5 +3123,6 @@ expression: default_configuration
       "varchar": "string"
     }
   },
-  "mutationsVersion": "v2"
+  "mutationsVersion": "v2",
+  "mutationsPrefix": ""
 }