no-op refactor: lift process_command_relationship_definition to ope…

…ndd planning (#1455)

<!-- The PR description should answer 2 important questions: -->

### What

Move `process_command_relationship_definition` to opendd planning code
path. This also requires the `LocalCommandRelationshipInfo` type to be
moved.

This is a no-op refactor and incremental PR towards supporting command
relationship in opendd query resolution.

### How

Just refactor and move stuff.

V3_GIT_ORIGIN_REV_ID: 1ef74c71e3be335e276873306637384cfb63e821

v3/crates/graphql/ir/src/lib.rs

@@ -34,9 +34,7 @@ pub use plan::{
     QueryPlan, RequestPlan,
 };
 pub use query_root::generate_ir as generate_query_ir;
-pub use relationship::{
-    build_remote_command_relationship, build_remote_relationship, LocalCommandRelationshipInfo,
-};
+pub use relationship::{build_remote_command_relationship, build_remote_relationship};
 pub use root_field::{
     ApolloFederationRootFields, MutationRootField, QueryRootField, SubscriptionRootField,
 };

v3/crates/graphql/ir/src/plan/error.rs

@@ -1,10 +1,3 @@
-use metadata_resolve::Qualified;
-use open_dds::{
-    arguments::ArgumentName,
-    commands::CommandName,
-    relationships::RelationshipName,
-    types::{CustomTypeName, FieldName},
-};
 use tracing_util::TraceableError;
 
 #[derive(Debug, thiserror::Error)]
@@ -33,20 +26,6 @@ impl TraceableError for Error {
 
 #[derive(Debug, thiserror::Error)]
 pub enum InternalError {
-    #[error("Mapping for source column {source_column} already exists in the relationship {relationship_name}")]
-    MappingExistsInRelationship {
-        source_column: FieldName,
-        relationship_name: RelationshipName,
-    },
-
-    #[error("Missing argument mapping to command {command_name} data connector source for argument {argument_name} used in relationship {relationship_name} on type {source_type}")]
-    MissingArgumentMappingInCommandRelationship {
-        source_type: Qualified<CustomTypeName>,
-        relationship_name: RelationshipName,
-        command_name: Qualified<CommandName>,
-        argument_name: ArgumentName,
-    },
-
     #[error("remote relationships should have been handled separately")]
     RemoteRelationshipsAreNotSupported,
 

v3/crates/graphql/ir/src/plan/relationships.rs

@@ -1,15 +1,11 @@
 //! NDC query generation from 'ModelSelection' IR for relationships.
 
-use open_dds::data_connector::CollectionName;
-use open_dds::relationships::RelationshipType;
-use open_dds::types::DataConnectorArgumentName;
 use std::collections::BTreeMap;
 
 use super::error;
-use crate::LocalCommandRelationshipInfo;
 use crate::ModelSelection;
 use plan::process_model_relationship_definition;
-use plan_types::{NdcRelationshipName, Relationship, RelationshipArgument};
+use plan_types::{NdcRelationshipName, Relationship};
 
 /// collect relationships from OrderBy IR component containing relationships.
 pub(crate) fn collect_relationships_from_order_by(
@@ -30,71 +26,3 @@ pub(crate) fn collect_relationships_from_order_by(
     };
     Ok(())
 }
-
-pub(crate) fn process_command_relationship_definition(
-    relationship_info: &LocalCommandRelationshipInfo,
-) -> Result<Relationship, error::Error> {
-    let &LocalCommandRelationshipInfo {
-        annotation,
-        source_data_connector: _,
-        source_type_mappings,
-        target_source,
-    } = relationship_info;
-
-    let mut arguments = BTreeMap::new();
-    for metadata_resolve::RelationshipCommandMapping {
-        source_field: source_field_path,
-        argument_name: target_argument,
-    } in &annotation.mappings
-    {
-        let source_column = metadata_resolve::get_field_mapping_of_field_name(
-            source_type_mappings,
-            &annotation.source_type,
-            &annotation.relationship_name,
-            &source_field_path.field_name,
-        )
-        .map_err(|e| error::InternalError::InternalGeneric {
-            description: e.to_string(),
-        })?;
-
-        let relationship_argument = RelationshipArgument::Column {
-            name: source_column.column,
-        };
-
-        let connector_argument_name = target_source
-            .details
-            .argument_mappings
-            .get(target_argument)
-            .ok_or_else(|| {
-                error::Error::Internal(
-                    error::InternalError::MissingArgumentMappingInCommandRelationship {
-                        source_type: annotation.source_type.clone(),
-                        relationship_name: annotation.relationship_name.clone(),
-                        command_name: annotation.command_name.clone(),
-                        argument_name: target_argument.clone(),
-                    },
-                )
-            })?;
-
-        if arguments
-            .insert(
-                DataConnectorArgumentName::from(connector_argument_name.as_str()),
-                relationship_argument,
-            )
-            .is_some()
-        {
-            Err(error::InternalError::MappingExistsInRelationship {
-                source_column: source_field_path.field_name.clone(),
-                relationship_name: annotation.relationship_name.clone(),
-            })?;
-        }
-    }
-
-    let relationship = Relationship {
-        column_mapping: BTreeMap::new(),
-        relationship_type: RelationshipType::Object,
-        target_collection: CollectionName::from(target_source.function_name.as_str()),
-        arguments,
-    };
-    Ok(relationship)
-}

v3/crates/graphql/ir/src/plan/selection_set.rs

@@ -1,7 +1,6 @@
 use super::arguments;
 use super::commands;
 use super::model_selection;
-use super::relationships;
 use super::types::Plan;
 use super::ProcessResponseAs;
 use crate::plan::error;
@@ -10,7 +9,7 @@ use indexmap::IndexMap;
 use metadata_resolve::data_connectors::NdcVersion;
 use metadata_resolve::FieldMapping;
 use open_dds::data_connector::DataConnectorColumnName;
-use plan::process_model_relationship_definition;
+use plan::{process_command_relationship_definition, process_model_relationship_definition};
 use plan_types::{
     ExecutionTree, Field, JoinLocations, JoinNode, Location, LocationKind, NestedArray,
     NestedField, NestedObject, PredicateQueryTrees, RemoteJoin, RemoteJoinType, SourceFieldAlias,
@@ -166,7 +165,13 @@ pub(crate) fn plan_selection_set(
                 // collect local command relationship
                 relationships.insert(
                     name.clone(),
-                    relationships::process_command_relationship_definition(relationship_info)?,
+                    process_command_relationship_definition(relationship_info).map_err(
+                        |plan_error| {
+                            error::Error::Internal(error::InternalError::InternalGeneric {
+                                description: plan_error.to_string(),
+                            })
+                        },
+                    )?,
                 );
                 let Plan {
                     inner: relationship_query,

v3/crates/graphql/ir/src/relationship.rs

@@ -25,24 +25,14 @@ use graphql_schema::{
     InputAnnotation, ModelAggregateRelationshipAnnotation, ModelInputAnnotation,
     ModelRelationshipAnnotation, GDS,
 };
-use metadata_resolve::{self, serialize_qualified_btreemap, Qualified, RelationshipModelMapping};
+use metadata_resolve::{self, Qualified, RelationshipModelMapping};
 use plan::{count_command, count_model};
 use plan_types::{
-    ComparisonTarget, ComparisonValue, Expression, LocalFieldComparison,
-    LocalModelRelationshipInfo, NdcRelationshipName, UsagesCounts, VariableName,
+    ComparisonTarget, ComparisonValue, Expression, LocalCommandRelationshipInfo,
+    LocalFieldComparison, LocalModelRelationshipInfo, NdcRelationshipName, UsagesCounts,
+    VariableName,
 };
 
-#[derive(Debug, Serialize)]
-pub struct LocalCommandRelationshipInfo<'s> {
-    pub annotation: &'s CommandRelationshipAnnotation,
-    pub source_data_connector: &'s metadata_resolve::DataConnectorLink,
-    #[serde(serialize_with = "serialize_qualified_btreemap")]
-    pub source_type_mappings:
-        &'s BTreeMap<Qualified<CustomTypeName>, metadata_resolve::TypeMapping>,
-
-    pub target_source: &'s CommandTargetSource,
-}
-
 #[derive(Debug, Clone, Serialize)]
 pub struct RemoteModelRelationshipInfo {
     /// This contains processed information about the mappings.
@@ -309,7 +299,6 @@ pub fn generate_command_relationship_ir<'s>(
             field,
             field_call,
             annotation,
-            source_data_connector,
             type_mappings,
             target_source,
             session_variables,
@@ -364,7 +353,6 @@ pub fn build_local_command_relationship<'s>(
     field: &normalized_ast::Field<'s, GDS>,
     field_call: &normalized_ast::FieldCall<'s, GDS>,
     annotation: &'s CommandRelationshipAnnotation,
-    data_connector: &'s metadata_resolve::DataConnectorLink,
     type_mappings: &'s BTreeMap<Qualified<CustomTypeName>, metadata_resolve::TypeMapping>,
     target_source: &'s CommandTargetSource,
     session_variables: &SessionVariables,
@@ -385,10 +373,13 @@ pub fn build_local_command_relationship<'s>(
     )?;
 
     let rel_info = LocalCommandRelationshipInfo {
-        annotation,
-        source_data_connector: data_connector,
+        relationship_name: &annotation.relationship_name,
+        source_type: &annotation.source_type,
         source_type_mappings: type_mappings,
-        target_source,
+        command_name: &annotation.command_name,
+        argument_mappings: &target_source.details.argument_mappings,
+        function_name: &target_source.function_name,
+        mappings: &annotation.mappings,
     };
 
     // Relationship names needs to be unique across the IR. This is so that, the

v3/crates/graphql/ir/src/selection_set.rs

@@ -11,15 +11,15 @@ use std::collections::BTreeMap;
 use super::arguments;
 use super::commands::FunctionBasedCommand;
 use super::model_selection::ModelSelection;
-use super::relationship::{
-    self, LocalCommandRelationshipInfo, RemoteCommandRelationshipInfo, RemoteModelRelationshipInfo,
-};
+use super::relationship::{self, RemoteCommandRelationshipInfo, RemoteModelRelationshipInfo};
 use crate::error;
 use crate::global_id;
 use graphql_schema::TypeKind;
 use graphql_schema::{Annotation, OutputAnnotation, RootFieldAnnotation, GDS};
 use plan::UnresolvedArgument;
-use plan_types::{LocalModelRelationshipInfo, NdcRelationshipName, UsagesCounts};
+use plan_types::{
+    LocalCommandRelationshipInfo, LocalModelRelationshipInfo, NdcRelationshipName, UsagesCounts,
+};
 
 #[derive(Debug, Serialize)]
 pub enum NestedSelection<'s> {

v3/crates/plan-types/src/lib.rs

@@ -25,6 +25,8 @@ pub use ndc_field_alias::NdcFieldAlias;
 pub use ndc_function_ir_value::FUNCTION_IR_VALUE_COLUMN_NAME;
 pub use ndc_relationship_name::NdcRelationshipName;
 pub use order_by::{OrderByDirection, OrderByElement, OrderByTarget};
-pub use relationships::{LocalModelRelationshipInfo, RelationshipPathElement};
+pub use relationships::{
+    LocalCommandRelationshipInfo, LocalModelRelationshipInfo, RelationshipPathElement,
+};
 pub use usage_counts::{CommandCount, ModelCount, UsagesCounts};
 pub use variable_name::VariableName;

v3/crates/plan-types/src/relationships.rs

@@ -1,8 +1,10 @@
 use metadata_resolve::{self, serialize_qualified_btreemap, Qualified};
 use open_dds::{
+    arguments::ArgumentName,
+    commands::{CommandName, FunctionName},
     data_connector::DataConnectorColumnName,
     relationships::{RelationshipName, RelationshipType},
-    types::CustomTypeName,
+    types::{CustomTypeName, DataConnectorArgumentName},
 };
 use serde::Serialize;
 use std::collections::BTreeMap;
@@ -29,3 +31,15 @@ pub struct RelationshipPathElement<TExpression> {
     pub relationship_name: NdcRelationshipName,
     pub filter_predicate: Option<TExpression>,
 }
+
+#[derive(Debug, Serialize, Clone, PartialEq)]
+pub struct LocalCommandRelationshipInfo<'s> {
+    pub relationship_name: &'s RelationshipName,
+    pub source_type: &'s Qualified<CustomTypeName>,
+    pub source_type_mappings:
+        &'s BTreeMap<Qualified<CustomTypeName>, metadata_resolve::TypeMapping>,
+    pub command_name: &'s Qualified<CommandName>,
+    pub argument_mappings: &'s BTreeMap<ArgumentName, DataConnectorArgumentName>,
+    pub function_name: &'s FunctionName,
+    pub mappings: &'s Vec<metadata_resolve::RelationshipCommandMapping>,
+}

v3/crates/plan/src/lib.rs

@@ -15,7 +15,8 @@ pub use query::{
     build_relationship_comparison_expression, execute_plan_from_function,
     execute_plan_from_procedure, from_command, from_model_aggregate_selection,
     from_model_selection, get_field_mapping_of_field_name, ndc_query_to_query_execution_plan,
-    plan_expression, plan_query_request, process_argument_presets, process_model_predicate,
+    plan_expression, plan_query_request, process_argument_presets,
+    process_command_relationship_definition, process_model_predicate,
     process_model_relationship_definition, CommandPlan, ExecutionPlan, FromCommand,
     ModelAggregateSelection, NDCFunction, NDCProcedure, NDCQuery, QueryExecution,
     SingleNodeExecutionPlan, UnresolvedArgument,

v3/crates/plan/src/query.rs

@@ -22,7 +22,9 @@ pub use model::{
     ModelAggregateSelection,
 };
 pub use permissions::process_model_predicate;
-pub use relationships::process_model_relationship_definition;
+pub use relationships::{
+    process_command_relationship_definition, process_model_relationship_definition,
+};
 use std::sync::Arc;
 pub use types::{NDCFunction, NDCProcedure, NDCQuery, QueryContext};
 

v3/crates/plan/src/query/field_selection.rs

@@ -309,12 +309,7 @@ fn from_relationship_selection(
         plan_types::NdcRelationshipName::new(object_type_name, &target.relationship_name);
     relationships.insert(
         ndc_relationship_name.clone(),
-        process_model_relationship_definition(&local_model_relationship_info).map_err(|err| {
-            PlanError::Internal(format!(
-                "Unable to process relationship {} definition: {}",
-                &target.relationship_name, err
-            ))
-        })?,
+        process_model_relationship_definition(&local_model_relationship_info)?,
     );
 
     let relationship_model_target = ModelTarget {