From 55cb567c07cd17f2dc9ab88d19ace83116867f95 Mon Sep 17 00:00:00 2001 From: Rakesh Emmadi <12475069+rakeshkky@users.noreply.github.com> Date: Mon, 20 Jan 2025 21:37:51 +0530 Subject: [PATCH] OpenDd execution: Implement order by for model and relationship selection in GraphQL frontend (#1550) ### What Implement order by input argument resolution for model (select_many and select_aggregate) and relationship selection in GraphQL for openDd IR conversion. V3_GIT_ORIGIN_REV_ID: a20dacf611ecaa2b86439392f6e97884a8dba5b3 --- v3/crates/engine/tests/execution.rs | 22 +- v3/crates/engine/tests/relationship.rs | 4 +- v3/crates/graphql/ir/src/commands.rs | 15 + v3/crates/graphql/ir/src/model_selection.rs | 19 +- v3/crates/graphql/ir/src/mutation_root.rs | 1 + v3/crates/graphql/ir/src/order_by.rs | 304 ++++++++++++++++++ v3/crates/graphql/ir/src/query_root.rs | 1 + .../ir/src/query_root/select_aggregate.rs | 15 +- .../graphql/ir/src/query_root/select_many.rs | 16 +- .../graphql/ir/src/query_root/select_one.rs | 6 +- v3/crates/graphql/ir/src/relationship.rs | 101 +++--- v3/crates/graphql/ir/src/selection_set.rs | 12 + v3/crates/open-dds/src/query.rs | 4 +- 13 files changed, 451 insertions(+), 69 deletions(-) diff --git a/v3/crates/engine/tests/execution.rs b/v3/crates/engine/tests/execution.rs index 7a11d535e52ff..358c950c7146c 100644 --- a/v3/crates/engine/tests/execution.rs +++ b/v3/crates/engine/tests/execution.rs @@ -228,7 +228,7 @@ fn test_model_select_many_order_by() -> anyhow::Result<()> { common::test_execution_expectation( test_path_string, &[common_metadata_path_string], - common::TestOpenDDPipeline::Skip, + common::TestOpenDDPipeline::YesPlease, ) } @@ -239,7 +239,7 @@ fn test_model_select_many_order_by_with_model_v2() -> anyhow::Result<()> { common::test_execution_expectation( test_path_string, &[common_metadata_path_string], - common::TestOpenDDPipeline::Skip, + common::TestOpenDDPipeline::YesPlease, ) } @@ -250,7 +250,7 @@ fn test_model_select_many_order_by_nested() -> anyhow::Result<()> { common::test_execution_expectation( test_path_string, &[common_metadata_path_string], - common::TestOpenDDPipeline::Skip, + common::TestOpenDDPipeline::YesPlease, ) } @@ -272,7 +272,7 @@ fn test_model_select_many_order_by_nested_legacy() -> anyhow::Result<()> { common::test_execution_expectation( test_path_string, &[common_metadata_path_string], - common::TestOpenDDPipeline::Skip, + common::TestOpenDDPipeline::YesPlease, ) } @@ -283,7 +283,7 @@ fn test_model_select_many_order_by_filter() -> anyhow::Result<()> { common::test_execution_expectation( test_path_string, &[common_metadata_path_string], - common::TestOpenDDPipeline::Skip, + common::TestOpenDDPipeline::YesPlease, ) } @@ -295,7 +295,7 @@ fn test_model_select_many_order_by_with_graphql_config() -> anyhow::Result<()> { common::test_execution_expectation( test_path_string, &[common_metadata_path_string, metadata_graphql_json], - common::TestOpenDDPipeline::Skip, + common::TestOpenDDPipeline::YesPlease, ) } @@ -313,7 +313,7 @@ fn test_model_select_many_order_by_multiple_columns() -> anyhow::Result<()> { common::test_execution_expectation( test_path_string, &[common_metadata_path_string], - common::TestOpenDDPipeline::Skip, + common::TestOpenDDPipeline::YesPlease, ) } @@ -324,7 +324,7 @@ fn test_model_select_many_order_by_multiple_columns_validation_check() -> anyhow common::test_execution_expectation( test_path_string, &[common_metadata_path_string], - common::TestOpenDDPipeline::Skip, + common::TestOpenDDPipeline::YesPlease, ) } @@ -337,7 +337,7 @@ fn test_model_select_many_order_by_multiple_nested_columns_validation_check() -> common::test_execution_expectation( test_path_string, &[common_metadata_path_string], - common::TestOpenDDPipeline::Skip, + common::TestOpenDDPipeline::YesPlease, ) } @@ -349,7 +349,7 @@ fn test_model_select_many_type_permission_order_by() -> anyhow::Result<()> { common::test_execution_expectation( test_path_string, &[common_metadata_path_string], - common::TestOpenDDPipeline::Skip, + common::TestOpenDDPipeline::YesPlease, ) } @@ -396,7 +396,7 @@ fn test_model_select_many_order_by_reuse_order_by_expression() -> anyhow::Result common::test_execution_expectation( test_path_string, &[common_metadata_path_string], - common::TestOpenDDPipeline::Skip, + common::TestOpenDDPipeline::YesPlease, ) } diff --git a/v3/crates/engine/tests/relationship.rs b/v3/crates/engine/tests/relationship.rs index 7dd3de80c65bd..2a2591576dc31 100644 --- a/v3/crates/engine/tests/relationship.rs +++ b/v3/crates/engine/tests/relationship.rs @@ -64,7 +64,7 @@ fn test_local_relationships_model_to_model_array_with_arguments() -> anyhow::Res common::test_execution_expectation( test_path_string, &[common_metadata_path_string], - common::TestOpenDDPipeline::Skip, + common::TestOpenDDPipeline::YesPlease, ) } @@ -77,7 +77,7 @@ fn test_relationships_array_with_arguments_with_graphql_config() -> anyhow::Resu common::test_execution_expectation( test_path_string, &[common_metadata_path_string, common_metadata_graphql_config], - common::TestOpenDDPipeline::Skip, + common::TestOpenDDPipeline::YesPlease, ) } diff --git a/v3/crates/graphql/ir/src/commands.rs b/v3/crates/graphql/ir/src/commands.rs index 2631a44229841..8fc7a3e4f50ef 100644 --- a/v3/crates/graphql/ir/src/commands.rs +++ b/v3/crates/graphql/ir/src/commands.rs @@ -182,6 +182,10 @@ pub fn generate_command_info<'n, 's>( /// Generates the OpenDD IR for a 'command' operation #[allow(irrefutable_let_patterns)] pub fn generate_command_info_open_dd<'n, 's>( + models: &'s IndexMap< + metadata_resolve::Qualified, + metadata_resolve::ModelWithPermissions, + >, command_name: &Qualified, field: &'n normalized_ast::Field<'s, GDS>, field_call: &'n normalized_ast::FieldCall<'s, GDS>, @@ -218,6 +222,7 @@ pub fn generate_command_info_open_dd<'n, 's>( result_type, result_base_type_kind, metadata_resolve::FieldNestedness::NotNested, + models, &command_source.type_mappings, selection_set::NestedSelectionType::CommandRootSelection, field, @@ -336,6 +341,10 @@ pub fn generate_function_based_command<'n, 's>( /// Generates the OpenDD IR for a 'function based command' operation pub fn generate_function_based_command_open_dd<'n, 's>( + models: &'s IndexMap< + metadata_resolve::Qualified, + metadata_resolve::ModelWithPermissions, + >, command_name: &Qualified, function_name: &'s open_dds::commands::FunctionName, field: &'n normalized_ast::Field<'s, GDS>, @@ -348,6 +357,7 @@ pub fn generate_function_based_command_open_dd<'n, 's>( usage_counts: &mut UsagesCounts, ) -> Result, error::Error> { let command_info = generate_command_info_open_dd( + models, command_name, field, field_call, @@ -417,6 +427,10 @@ pub fn generate_procedure_based_command<'n, 's>( /// Generates the OpenDD IR for a 'procedure based command' operation pub fn generate_procedure_based_command_open_dd<'n, 's>( + models: &'s IndexMap< + metadata_resolve::Qualified, + metadata_resolve::ModelWithPermissions, + >, command_name: &Qualified, procedure_name: &'s open_dds::commands::ProcedureName, field: &'n normalized_ast::Field<'s, GDS>, @@ -430,6 +444,7 @@ pub fn generate_procedure_based_command_open_dd<'n, 's>( let mut usage_counts = UsagesCounts::new(); let command_info = generate_command_info_open_dd( + models, command_name, field, field_call, diff --git a/v3/crates/graphql/ir/src/model_selection.rs b/v3/crates/graphql/ir/src/model_selection.rs index aa10ee1fba5f0..adea72146608a 100644 --- a/v3/crates/graphql/ir/src/model_selection.rs +++ b/v3/crates/graphql/ir/src/model_selection.rs @@ -71,12 +71,17 @@ struct FilterInputArguments<'s> { pub fn model_selection_open_dd_ir( selection_set: &normalized_ast::SelectionSet<'_, GDS>, model_name: &Qualified, + models: &IndexMap< + metadata_resolve::Qualified, + metadata_resolve::ModelWithPermissions, + >, type_mappings: &BTreeMap< metadata_resolve::Qualified, metadata_resolve::TypeMapping, >, model_arguments: Option>, where_clause: Option, + order_by: Vec, limit: Option, offset: Option, session_variables: &SessionVariables, @@ -86,6 +91,7 @@ pub fn model_selection_open_dd_ir( let selection = selection_set::generate_selection_set_open_dd_ir( selection_set, metadata_resolve::FieldNestedness::NotNested, + models, type_mappings, session_variables, request_headers, @@ -107,12 +113,6 @@ pub fn model_selection_open_dd_ir( // TODO: append select permissions etc let filter = where_clause; - // MADE UP EMPTY VALUES TO GET THINGS STARTED - // TODO: these will be replaced with correct values as we go - let order_by = vec![]; - - // END OF MADE UP VALUES - let target = open_dds::query::ModelTarget { subgraph: model_name.subgraph.clone(), model_name: model_name.name.clone(), @@ -135,6 +135,7 @@ pub fn model_aggregate_selection_open_dd_ir<'s>( model_name: &Qualified, model_arguments: Option>, where_clause: Option, + order_by: Vec, limit: Option, offset: Option, usage_counts: &mut UsagesCounts, @@ -158,12 +159,6 @@ pub fn model_aggregate_selection_open_dd_ir<'s>( // TODO: append select permissions etc let filter = where_clause; - // MADE UP EMPTY VALUES TO GET THINGS STARTED - // TODO: these will be replaced with correct values as we go - let order_by = vec![]; - - // END OF MADE UP VALUES - let target = open_dds::query::ModelTarget { subgraph: model_name.subgraph.clone(), model_name: model_name.name.clone(), diff --git a/v3/crates/graphql/ir/src/mutation_root.rs b/v3/crates/graphql/ir/src/mutation_root.rs index 4586f9c04fae4..298c9f88131a5 100644 --- a/v3/crates/graphql/ir/src/mutation_root.rs +++ b/v3/crates/graphql/ir/src/mutation_root.rs @@ -89,6 +89,7 @@ pub fn generate_ir<'n, 's>( } GraphqlRequestPipeline::OpenDd => { commands::generate_procedure_based_command_open_dd( + &metadata.models, name, procedure_name, field, diff --git a/v3/crates/graphql/ir/src/order_by.rs b/v3/crates/graphql/ir/src/order_by.rs index 1c3da91574177..044d2463ba2f5 100644 --- a/v3/crates/graphql/ir/src/order_by.rs +++ b/v3/crates/graphql/ir/src/order_by.rs @@ -4,6 +4,7 @@ use std::ops::Deref; use graphql_schema::OrderByRelationshipAnnotation; use graphql_schema::{Annotation, InputAnnotation, ModelInputAnnotation}; use hasura_authn_core::SessionVariables; +use indexmap::IndexMap; use lang_graphql::normalized_ast::{self as normalized_ast, InputField, Value}; use open_dds::data_connector::DataConnectorColumnName; use plan::count_model; @@ -357,3 +358,306 @@ pub fn build_ndc_order_by_element<'s>( Ok(order_by_elements) } + +pub fn build_order_by_open_dd_ir<'s>( + args_input: &Value<'s, GDS>, + usage_counts: &mut UsagesCounts, + data_connector_link: &'s metadata_resolve::DataConnectorLink, + data_type: &metadata_resolve::Qualified, +) -> Result, error::Error> { + match args_input { + normalized_ast::Value::List(arguments) => { + let mut order_by_elements = Vec::new(); + for argument in arguments { + let order_by_element = build_order_by_element_open_dd_ir( + argument, + metadata_resolve::MultipleOrderByInputObjectFields::Disallow, + usage_counts, + data_connector_link, + data_type, + )?; + order_by_elements.extend(order_by_element); + } + Ok(order_by_elements) + } + _ => Err(error::InternalEngineError::InternalGeneric { + description: "Expected list of input objects value for order_by".into(), + })?, + } +} + +// Build the OpenDD OrderByElement by traversing the relationships when present +// For eg: If we have the following order_by query: +// Track(order_by: {Album: {Artist: {ArtistId: Asc}, AlbumId: Asc}}, limit: 15) +// where, '{Album: {Artist: {ArtistId: Asc}, AlbumId: Asc}}' will be annotated as 'ModelOrderByRelationshipArgument' +// the `OrderByElement` will be: +// [ +// open_dds::query::OrderByElement { +// direction: Asc, +// operand: Relationship( +// RelationshipOperand { +// target: RelationshipTarget { +// relationship_name: "Album", +// arguments: {}, +// filter: None, +// order_by: [], +// limit: None, +// offset: None, +// }, +// nested: Some(Relationship( +// RelationshipOperand { +// target: RelationshipTarget { +// relationship_name: "Artist", +// arguments: {}, +// filter: None, +// order_by: [], +// limit: None, +// offset: None, +// }, +// nested: Some(Field( +// ObjectFieldOperand { +// target: ObjectFieldTarget { +// field_name: "ArtistId", +// arguments: {}, +// }, +// nested: None, +// } +// )), +// } +// )), +// } +// ), +// }, +// open_dds::query::OrderByElement { +// direction: Asc, +// operand: Relationship( +// RelationshipOperand { +// target: RelationshipTarget { +// relationship_name: "Album", +// arguments: {}, +// filter: None, +// order_by: [], +// limit: None, +// offset: None, +// }, +// nested: Some(Field( +// ObjectFieldOperand { +// target: ObjectFieldTarget { +// field_name: "AlbumId", +// arguments: {}, +// }, +// nested: None, +// } +// )), +// } +// ), +// }, +// ] + +pub fn build_order_by_element_open_dd_ir<'s>( + input_field_value: &Value<'s, GDS>, + multiple_input_fields: metadata_resolve::MultipleOrderByInputObjectFields, + usage_counts: &mut UsagesCounts, + data_connector_link: &'s metadata_resolve::DataConnectorLink, + data_type: &metadata_resolve::Qualified, +) -> Result, error::Error> { + let input_object_fields = input_field_value.as_object()?; + let mut order_by_elements = Vec::new(); + + // Technically this function should only ever return a single order by element, but thanks to a bug + // that we have to retain the behaviour of for backwards compatibility, we sometimes need to allow + // returning multiple order by elements. + // We don't actually want to support returning multiple order by elements based on a user having + // multiple input object fields in a single order_by object, because the order of these fields is + // undefined in GraphQL. So we need to force them to specify all ordering elements in a top level list. + if multiple_input_fields == metadata_resolve::MultipleOrderByInputObjectFields::Disallow + && input_object_fields.len() > 1 + { + return Err(error::Error::OrderByObjectShouldExactlyHaveOneKeyValuePair); + } + + for object_field in input_object_fields.values() { + match object_field.info.generic { + // The column that we want to use for ordering. If the column happens to be + // a relationship column, we'll have to join all the paths to specify NDC, + // what relationships needs to be traversed to access this column + Annotation::Input(InputAnnotation::Model( + graphql_schema::ModelInputAnnotation::ModelOrderByArgument { + parent_type: _, + field_name, + .. + }, + )) => { + let order_by_value = object_field.value.as_enum()?; + let order_direction = match &order_by_value.info.generic { + Annotation::Input(InputAnnotation::Model( + ModelInputAnnotation::ModelOrderByDirection { direction }, + )) => direction, + &annotation => { + return Err(error::InternalEngineError::UnexpectedAnnotation { + annotation: annotation.clone(), + })? + } + }; + let operand = + open_dds::query::Operand::Field(open_dds::query::ObjectFieldOperand { + target: Box::new(open_dds::query::ObjectFieldTarget { + field_name: field_name.clone(), + arguments: IndexMap::new(), + }), + nested: None, + }); + let direction = match order_direction { + graphql_schema::ModelOrderByDirection::Asc => { + open_dds::models::OrderByDirection::Asc + } + graphql_schema::ModelOrderByDirection::Desc => { + open_dds::models::OrderByDirection::Desc + } + }; + + let order_element = open_dds::query::OrderByElement { direction, operand }; + + order_by_elements.push(order_element); + } + // A relationship is being used to order the results. This relationship can + // either point to another relationship or a column. + Annotation::Input(InputAnnotation::Model( + graphql_schema::ModelInputAnnotation::ModelOrderByRelationshipArgument( + OrderByRelationshipAnnotation { + relationship_name, + relationship_type: _, + source_type: _, + object_type_name, + target_source, + target_type: _, + target_model_name, + mappings: _, + deprecated: _, + multiple_input_properties, + }, + ), + )) => { + let relationship_field_nestedness = if *data_type == *object_type_name { + metadata_resolve::FieldNestedness::NotNested + } else { + metadata_resolve::FieldNestedness::ObjectNested + }; + if let metadata_resolve::RelationshipExecutionCategory::Local = + metadata_resolve::relationship_execution_category( + relationship_field_nestedness, + data_connector_link, + &target_source.model.data_connector, + &target_source.capabilities, + ) + { + // Add the target model being used in the usage counts + count_model(target_model_name, usage_counts); + + let relationship_target = open_dds::query::RelationshipTarget { + relationship_name: relationship_name.clone(), + arguments: IndexMap::new(), + // Following parameters are not applicable in the order_by input. + filter: None, // NOTE: Permission filters are handled during OpenDd query planning. + order_by: vec![], + limit: None, + offset: None, + }; + + let new_order_by_elements = build_order_by_element_open_dd_ir( + &object_field.value, + *multiple_input_properties, + usage_counts, + data_connector_link, + data_type, + )?; + + for element in new_order_by_elements { + let operand = open_dds::query::Operand::Relationship( + open_dds::query::RelationshipOperand { + target: Box::new(relationship_target.clone()), + nested: Some(Box::new(element.operand)), + }, + ); + order_by_elements.push(open_dds::query::OrderByElement { + direction: element.direction, + operand, + }); + } + } else { + Err(error::InternalEngineError::InternalGeneric { + description: "Remote relationships are not supported in order_by".into(), + })?; + } + } + Annotation::Input(InputAnnotation::Model( + graphql_schema::ModelInputAnnotation::ModelOrderByNestedExpression { + parent_type: _, + field_name, + multiple_input_properties, + }, + )) => { + match multiple_input_properties { + metadata_resolve::MultipleOrderByInputObjectFields::Disallow => { + let new_order_by_elements = build_order_by_element_open_dd_ir( + &object_field.value, + *multiple_input_properties, + usage_counts, + data_connector_link, + data_type, + )?; + + for element in new_order_by_elements { + let operand = open_dds::query::Operand::Field( + open_dds::query::ObjectFieldOperand { + target: Box::new(open_dds::query::ObjectFieldTarget { + field_name: field_name.clone(), + arguments: IndexMap::new(), + }), + nested: Some(Box::new(element.operand)), + }, + ); + order_by_elements.push(open_dds::query::OrderByElement { + direction: element.direction, + operand, + }); + } + } + metadata_resolve::MultipleOrderByInputObjectFields::Allow => { + // When multiple input properties are allowed for backwards compatibility purposes + // we expect nested properties to be (incorrectly!) typed as a list in graphql + let argument_list = object_field.value.as_list()?; + for argument in argument_list { + let new_order_by_elements = build_order_by_element_open_dd_ir( + argument, + *multiple_input_properties, + usage_counts, + data_connector_link, + data_type, + )?; + for element in new_order_by_elements { + let operand = open_dds::query::Operand::Field( + open_dds::query::ObjectFieldOperand { + target: Box::new(open_dds::query::ObjectFieldTarget { + field_name: field_name.clone(), + arguments: IndexMap::new(), + }), + nested: Some(Box::new(element.operand)), + }, + ); + order_by_elements.push(open_dds::query::OrderByElement { + direction: element.direction, + operand, + }); + } + } + } + } + } + annotation => Err(error::InternalEngineError::UnexpectedAnnotation { + annotation: annotation.clone(), + })?, + } + } + Ok(order_by_elements) +} diff --git a/v3/crates/graphql/ir/src/query_root.rs b/v3/crates/graphql/ir/src/query_root.rs index 7824f2c65266e..bed1b5e3720f6 100644 --- a/v3/crates/graphql/ir/src/query_root.rs +++ b/v3/crates/graphql/ir/src/query_root.rs @@ -345,6 +345,7 @@ fn generate_command_rootfield_ir<'n, 's>( &mut usage_counts, )?, GraphqlRequestPipeline::OpenDd => commands::generate_function_based_command_open_dd( + models, name, function_name, field, diff --git a/v3/crates/graphql/ir/src/query_root/select_aggregate.rs b/v3/crates/graphql/ir/src/query_root/select_aggregate.rs index 33f1410798d53..04347f53bbf37 100644 --- a/v3/crates/graphql/ir/src/query_root/select_aggregate.rs +++ b/v3/crates/graphql/ir/src/query_root/select_aggregate.rs @@ -21,6 +21,7 @@ use crate::arguments; use crate::error; use crate::filter; use crate::model_selection; +use crate::order_by; use crate::GraphqlRequestPipeline; #[derive(Debug, Serialize)] @@ -86,6 +87,7 @@ pub(crate) fn select_aggregate_generate_ir<'n, 's>( let mut offset = None; let mut where_input = None; let mut model_arguments_input = None; + let mut order_by_input = None; // Add the name of the root model count_model(model_name, &mut usage_counts); @@ -151,7 +153,7 @@ pub(crate) fn select_aggregate_generate_ir<'n, 's>( Annotation::Input(InputAnnotation::Model( ModelInputAnnotation::ModelOrderByExpression, )) => { - //TODO: Handle order_by + order_by_input = Some(&filter_input_field_arg.value); } // Where argument @@ -210,6 +212,16 @@ pub(crate) fn select_aggregate_generate_ir<'n, 's>( }) .transpose()?; + let order_by = match order_by_input { + None => vec![], + Some(order_by_input) => order_by::build_order_by_open_dd_ir( + order_by_input, + &mut usage_counts, + &model_source.data_connector, + data_type, + )?, + }; + ModelSelectAggregateSelection::OpenDd( model_selection::model_aggregate_selection_open_dd_ir( &field.selection_set, @@ -218,6 +230,7 @@ pub(crate) fn select_aggregate_generate_ir<'n, 's>( model_name, model_arguments, where_clause, + order_by, limit, offset, &mut usage_counts, diff --git a/v3/crates/graphql/ir/src/query_root/select_many.rs b/v3/crates/graphql/ir/src/query_root/select_many.rs index 85ec7c744f61f..4b6577fd8d8ba 100644 --- a/v3/crates/graphql/ir/src/query_root/select_many.rs +++ b/v3/crates/graphql/ir/src/query_root/select_many.rs @@ -16,7 +16,7 @@ use crate::arguments; use crate::error; use crate::filter; use crate::model_selection; -use crate::order_by::build_ndc_order_by; +use crate::order_by::{build_ndc_order_by, build_order_by_open_dd_ir}; use crate::permissions; use crate::GraphqlRequestPipeline; use graphql_schema::GDS; @@ -81,6 +81,7 @@ pub fn select_many_generate_ir<'n, 's>( // For opendd execution pipeline let mut model_arguments_input = None; + let mut order_by_input = None; // Add the name of the root model let mut usage_counts = UsagesCounts::new(); @@ -136,6 +137,8 @@ pub fn select_many_generate_ir<'n, 's>( &model_source.data_connector, data_type, )?); + // For opendd execution pipeline + order_by_input = Some(&argument.value); } _ => { return Err(error::InternalEngineError::UnexpectedAnnotation { @@ -189,13 +192,24 @@ pub fn select_many_generate_ir<'n, 's>( ) }) .transpose()?; + let order_by = match order_by_input { + None => vec![], + Some(order_by_input) => build_order_by_open_dd_ir( + order_by_input, + &mut usage_counts, + &model_source.data_connector, + data_type, + )?, + }; ModelSelectManySelection::OpenDd(model_selection::model_selection_open_dd_ir( &field.selection_set, model_name, + models, &model_source.type_mappings, model_arguments, where_clause, + order_by, limit, offset, &session.variables, diff --git a/v3/crates/graphql/ir/src/query_root/select_one.rs b/v3/crates/graphql/ir/src/query_root/select_one.rs index d866fc922e08a..5339a98495c3c 100644 --- a/v3/crates/graphql/ir/src/query_root/select_one.rs +++ b/v3/crates/graphql/ir/src/query_root/select_one.rs @@ -174,11 +174,13 @@ pub fn select_one_generate_ir<'n, 's>( ModelSelectOneSelection::OpenDd(model_selection::model_selection_open_dd_ir( &field.selection_set, model_name, + models, &model_source.type_mappings, Some(model_arguments), where_clause, - None, // limit - None, // offset + vec![], // order_by + None, // limit + None, // offset &session.variables, request_headers, // Get all the models/commands that were used as relationships diff --git a/v3/crates/graphql/ir/src/relationship.rs b/v3/crates/graphql/ir/src/relationship.rs index ebb8a63ac258d..161a5dd6934d5 100644 --- a/v3/crates/graphql/ir/src/relationship.rs +++ b/v3/crates/graphql/ir/src/relationship.rs @@ -20,6 +20,7 @@ use super::{ selection_set::{generate_selection_set_open_dd_ir, FieldSelection}, }; use crate::error; +use crate::order_by; use graphql_schema::{ Annotation, BooleanExpressionAnnotation, CommandRelationshipAnnotation, InputAnnotation, ModelAggregateRelationshipAnnotation, ModelInputAnnotation, ModelRelationshipAnnotation, GDS, @@ -52,6 +53,10 @@ pub type TargetField = (FieldName, metadata_resolve::NdcColumnForComparison); pub fn generate_model_relationship_open_dd_ir<'s>( field: &Field<'s, GDS>, + models: &'s IndexMap< + Qualified, + metadata_resolve::ModelWithPermissions, + >, type_mappings: &'s BTreeMap, metadata_resolve::TypeMapping>, relationship_annotation: &'s ModelRelationshipAnnotation, session_variables: &SessionVariables, @@ -65,55 +70,74 @@ pub fn generate_model_relationship_open_dd_ir<'s>( let mut limit = None; let mut offset = None; let mut where_input = None; + let mut order_by = Vec::new(); for argument in field_call.arguments.values() { match argument.info.generic { - annotation @ Annotation::Input(argument_annotation) => { - match argument_annotation { - InputAnnotation::Model(model_argument_annotation) => { - match model_argument_annotation { - ModelInputAnnotation::ModelLimitArgument => { - limit = Some(argument.value.as_int_u32().map_err( + annotation @ Annotation::Input(argument_annotation) => match argument_annotation { + InputAnnotation::Model(model_argument_annotation) => { + match model_argument_annotation { + ModelInputAnnotation::ModelLimitArgument => { + limit = + Some(argument.value.as_int_u32().map_err( error::Error::map_unexpected_value_to_external_error, )?); - } - ModelInputAnnotation::ModelOffsetArgument => { - offset = Some(argument.value.as_int_u32().map_err( + } + ModelInputAnnotation::ModelOffsetArgument => { + offset = + Some(argument.value.as_int_u32().map_err( error::Error::map_unexpected_value_to_external_error, )?); - } - ModelInputAnnotation::ModelOrderByExpression => { - /* - order_by = Some(build_ndc_order_by( - argument, - session_variables, - usage_counts, - )?);*/ - } - _ => { - return Err(error::InternalEngineError::UnexpectedAnnotation { - annotation: annotation.clone(), - })? - } } - } - InputAnnotation::BooleanExpression( - BooleanExpressionAnnotation::BooleanExpressionRootField, - ) => { - if let Some(_target_capabilities) = - &relationship_annotation.target_capabilities - { - where_input = Some(argument.value.as_object()?); + ModelInputAnnotation::ModelOrderByExpression => { + let target_model = models + .get(&relationship_annotation.model_name) + .ok_or_else(|| { + error::InternalError::Developer( + error::InternalDeveloperError::TargetModelNotFoundForRelationship { + model_name: relationship_annotation.model_name.clone(), + relationship_name: relationship_annotation.relationship_name.clone(), + }, + ) + })?; + let target_model_source = + target_model.model.source.as_ref().ok_or_else(|| { + error::Error::InternalMissingTargetModelSourceForRelationship { + relationship_name: relationship_annotation + .relationship_name + .clone(), + type_name: relationship_annotation.source_type.clone(), + } + })?; + order_by.extend(order_by::build_order_by_open_dd_ir( + &argument.value, + usage_counts, + &target_model_source.data_connector, + &relationship_annotation.target_type, + )?); + } + _ => { + return Err(error::InternalEngineError::UnexpectedAnnotation { + annotation: annotation.clone(), + })? } } - - _ => { - return Err(error::InternalEngineError::UnexpectedAnnotation { - annotation: annotation.clone(), - })? + } + InputAnnotation::BooleanExpression( + BooleanExpressionAnnotation::BooleanExpressionRootField, + ) => { + if let Some(_target_capabilities) = &relationship_annotation.target_capabilities + { + where_input = Some(argument.value.as_object()?); } } - } + + _ => { + return Err(error::InternalEngineError::UnexpectedAnnotation { + annotation: annotation.clone(), + })? + } + }, annotation => { return Err(error::InternalEngineError::UnexpectedAnnotation { @@ -135,6 +159,7 @@ pub fn generate_model_relationship_open_dd_ir<'s>( let selection = generate_selection_set_open_dd_ir( &field.selection_set, metadata_resolve::FieldNestedness::NotNested, + models, type_mappings, session_variables, request_headers, @@ -162,7 +187,7 @@ pub fn generate_model_relationship_open_dd_ir<'s>( filter, limit, offset, - order_by: vec![], // TODO: don't ignore me + order_by, }; // we're not worrying about local / remote relationships at this point, we'll see if that's diff --git a/v3/crates/graphql/ir/src/selection_set.rs b/v3/crates/graphql/ir/src/selection_set.rs index cdd023f4183c7..e4fdd62a772a4 100644 --- a/v3/crates/graphql/ir/src/selection_set.rs +++ b/v3/crates/graphql/ir/src/selection_set.rs @@ -136,6 +136,10 @@ pub fn generate_nested_selection_open_dd_ir( qualified_type_reference: &metadata_resolve::QualifiedTypeReference, field_base_type_kind: TypeKind, selection_set_field_nestedness: metadata_resolve::FieldNestedness, + models: &IndexMap< + metadata_resolve::Qualified, + metadata_resolve::ModelWithPermissions, + >, type_mappings: &BTreeMap< metadata_resolve::Qualified, metadata_resolve::TypeMapping, @@ -166,6 +170,7 @@ pub fn generate_nested_selection_open_dd_ir( element_type, field_base_type_kind, new_nestedness, + models, type_mappings, NestedSelectionType::NestedSelection, field, @@ -185,6 +190,7 @@ pub fn generate_nested_selection_open_dd_ir( let nested_selection = generate_selection_set_open_dd_ir( &field.selection_set, selection_set_field_nestedness, + models, type_mappings, session_variables, request_headers, @@ -297,6 +303,10 @@ pub fn generate_nested_selection<'s>( pub fn generate_selection_set_open_dd_ir( selection_set: &normalized_ast::SelectionSet<'_, GDS>, selection_set_field_nestedness: metadata_resolve::FieldNestedness, + models: &IndexMap< + metadata_resolve::Qualified, + metadata_resolve::ModelWithPermissions, + >, type_mappings: &BTreeMap< metadata_resolve::Qualified, metadata_resolve::TypeMapping, @@ -323,6 +333,7 @@ pub fn generate_selection_set_open_dd_ir( *field_base_type_kind, selection_set_field_nestedness .max(metadata_resolve::FieldNestedness::ObjectNested), + models, type_mappings, NestedSelectionType::NestedSelection, field, @@ -414,6 +425,7 @@ pub fn generate_selection_set_open_dd_ir( open_dds::query::ObjectSubSelection::Relationship( relationship::generate_model_relationship_open_dd_ir( field, + models, type_mappings, relationship_annotation, session_variables, diff --git a/v3/crates/open-dds/src/query.rs b/v3/crates/open-dds/src/query.rs index 350ea2d6c586b..8b7a8f2b509b7 100644 --- a/v3/crates/open-dds/src/query.rs +++ b/v3/crates/open-dds/src/query.rs @@ -416,8 +416,8 @@ impl ObjectFieldOperand { /// Operand targeting a particular relationship or an operand nested within that relationship. pub struct RelationshipOperand { #[serde(flatten)] - target: Box, - nested: Option>, + pub target: Box, + pub nested: Option>, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]