From 6b781009f12d8d4b2ee2686283a80669ae8c7f1b Mon Sep 17 00:00:00 2001 From: Daniel Harvey Date: Mon, 2 Dec 2024 09:59:16 +0000 Subject: [PATCH] Use counter instead of `uuid` in remote predicates (#1401) ### What We need a unique identifier for remote predicates, and used `uuid`. However this makes snapshot tests of `explain` unstable because the ordering changes all the time, so we switch it for a counter, threaded through the planning code. ### How Make a counter with a private inner value that we pass around, that provides a fresh `u64` when required. V3_GIT_ORIGIN_REV_ID: 386c6d47935b8da497e1471a3ca44bd52a12f72a --- v3/Cargo.lock | 1 - v3/crates/execute/src/error.rs | 3 +- v3/crates/execute/src/execute.rs | 7 ++- .../execute/src/execute/remote_predicates.rs | 19 ++++---- v3/crates/graphql/ir/src/plan.rs | 41 ++++++++++------ v3/crates/graphql/ir/src/plan/arguments.rs | 26 +++++++--- v3/crates/graphql/ir/src/plan/commands.rs | 21 +++++++-- v3/crates/graphql/ir/src/plan/filter.rs | 47 +++++++++++++++---- .../graphql/ir/src/plan/model_selection.rs | 15 ++++-- .../graphql/ir/src/plan/selection_set.rs | 31 ++++++++---- v3/crates/plan-types/Cargo.toml | 1 - v3/crates/plan-types/src/execution_plan.rs | 1 + .../plan-types/src/execution_plan/filter.rs | 5 +- .../plan-types/src/execution_plan/query.rs | 37 +++++++++++++-- v3/crates/plan-types/src/lib.rs | 2 +- 15 files changed, 184 insertions(+), 73 deletions(-) diff --git a/v3/Cargo.lock b/v3/Cargo.lock index 9bea1a87b36..a11770fa215 100644 --- a/v3/Cargo.lock +++ b/v3/Cargo.lock @@ -4177,7 +4177,6 @@ dependencies = [ "serde", "serde_json", "smol_str", - "uuid", ] [[package]] diff --git a/v3/crates/execute/src/error.rs b/v3/crates/execute/src/error.rs index 292c0ce50f7..2a54f6f69fa 100644 --- a/v3/crates/execute/src/error.rs +++ b/v3/crates/execute/src/error.rs @@ -2,6 +2,7 @@ use engine_types::ExposeInternalErrors; use gql::{ast::common as ast, http::GraphQLError}; use lang_graphql as gql; use open_dds::relationships::RelationshipName; +use plan_types::RemotePredicateKey; use reqwest::StatusCode; use serde_json as json; use thiserror::Error; @@ -218,7 +219,7 @@ pub enum FilterPredicateError { TooManyRowsReturned, #[error("could not find remote predicate result for {0}")] - CouldNotFindRemotePredicate(uuid::Uuid), + CouldNotFindRemotePredicate(RemotePredicateKey), } impl TraceableError for FilterPredicateError { diff --git a/v3/crates/execute/src/execute.rs b/v3/crates/execute/src/execute.rs index e7622b6d58b..7f97bcd9f29 100644 --- a/v3/crates/execute/src/execute.rs +++ b/v3/crates/execute/src/execute.rs @@ -3,7 +3,6 @@ // frontend use std::sync::Arc; -use uuid::Uuid; mod ndc_request; mod remote_joins; mod remote_predicates; @@ -15,7 +14,7 @@ pub use ndc_request::{make_ndc_mutation_request, make_ndc_query_request}; use plan_types::{ ExecutionTree, JoinLocations, NDCMutationExecution, NDCQueryExecution, NDCSubscriptionExecution, PredicateQueryTrees, ProcessResponseAs, QueryExecutionPlan, - ResolvedFilterExpression, + RemotePredicateKey, ResolvedFilterExpression, }; use std::collections::BTreeMap; @@ -53,7 +52,7 @@ async fn execute_remote_predicates( execution_span_attribute: &'static str, process_response_as: &ProcessResponseAs, project_id: Option<&ProjectId>, -) -> Result, FieldError> { +) -> Result, FieldError> { // resolve all our filter expressions into here, ready to && them in // at the appropriate moment let mut filter_expressions = BTreeMap::new(); @@ -109,7 +108,7 @@ async fn execute_execution_tree<'s>( execution_span_attribute: &'static str, process_response_as: &ProcessResponseAs, project_id: Option<&ProjectId>, - child_filter_expressions: &BTreeMap, + child_filter_expressions: &BTreeMap, ) -> Result, FieldError> { // given an execution tree // 1) run remote predicates diff --git a/v3/crates/execute/src/execute/remote_predicates.rs b/v3/crates/execute/src/execute/remote_predicates.rs index 6de401bb4af..cf183de69d1 100644 --- a/v3/crates/execute/src/execute/remote_predicates.rs +++ b/v3/crates/execute/src/execute/remote_predicates.rs @@ -2,17 +2,16 @@ use crate::error::{FieldError, FieldInternalError, FilterPredicateError}; use indexmap::{IndexMap, IndexSet}; use plan_types::{ Argument, Field, FieldsSelection, NestedArray, NestedField, NestedObject, QueryExecutionPlan, - QueryNodeNew, ResolvedFilterExpression, + QueryNodeNew, RemotePredicateKey, ResolvedFilterExpression, }; use std::collections::BTreeMap; -use uuid::Uuid; // replace any placeholders in our predicates with // `ResolvedFilterExpression`s we have calculated from // our remote predicate execution pub fn replace_predicates_in_query_execution_plan( query_execution_plan: QueryExecutionPlan, - predicates: &BTreeMap, + predicates: &BTreeMap, ) -> Result { Ok(QueryExecutionPlan { query_node: replace_predicates_in_query_node(query_execution_plan.query_node, predicates)?, @@ -35,7 +34,7 @@ pub fn replace_predicates_in_query_execution_plan( fn replace_predicates_in_argument( argument: Argument, - predicates: &BTreeMap, + predicates: &BTreeMap, ) -> Result { Ok(match argument { Argument::BooleanExpression { predicate } => Argument::BooleanExpression { @@ -48,7 +47,7 @@ fn replace_predicates_in_argument( fn replace_predicates_in_query_node( query_node: QueryNodeNew, - predicates: &BTreeMap, + predicates: &BTreeMap, ) -> Result { Ok(QueryNodeNew { aggregates: query_node.aggregates, @@ -78,7 +77,7 @@ fn replace_predicates_in_query_node( fn replace_predicates_in_nested_field( nested_field: NestedField, - predicates: &BTreeMap, + predicates: &BTreeMap, ) -> Result { Ok(match nested_field { NestedField::Object(nested_object) => NestedField::Object( @@ -92,7 +91,7 @@ fn replace_predicates_in_nested_field( } fn replace_predicates_in_nested_array( nested_array: NestedArray, - predicates: &BTreeMap, + predicates: &BTreeMap, ) -> Result { let NestedArray { fields } = nested_array; Ok(NestedArray { @@ -102,7 +101,7 @@ fn replace_predicates_in_nested_array( fn replace_predicates_in_nested_object( nested_object: NestedObject, - predicates: &BTreeMap, + predicates: &BTreeMap, ) -> Result { let NestedObject { fields } = nested_object; Ok(NestedObject { @@ -120,7 +119,7 @@ fn replace_predicates_in_nested_object( fn replace_predicates_in_field( field: Field, - predicates: &BTreeMap, + predicates: &BTreeMap, ) -> Result { Ok(match field { Field::Relationship { @@ -164,7 +163,7 @@ fn replace_predicates_in_field( fn replace_predicates_in_filter_expression( filter_expression: ResolvedFilterExpression, - predicates: &BTreeMap, + predicates: &BTreeMap, ) -> Result { Ok(match filter_expression { ResolvedFilterExpression::RemoteRelationshipComparison { diff --git a/v3/crates/graphql/ir/src/plan.rs b/v3/crates/graphql/ir/src/plan.rs index fb950aeed47..8df79503063 100644 --- a/v3/crates/graphql/ir/src/plan.rs +++ b/v3/crates/graphql/ir/src/plan.rs @@ -16,7 +16,7 @@ use indexmap::IndexMap; use lang_graphql as gql; use plan_types::{ ExecutionTree, NDCMutationExecution, NDCQueryExecution, NDCSubscriptionExecution, - ProcessResponseAs, QueryExecutionPlan, + ProcessResponseAs, QueryExecutionPlan, UniqueNumber, }; pub use relationships::process_model_relationship_definition; pub use types::{ @@ -36,11 +36,13 @@ pub use types::{ pub fn generate_request_plan<'n, 's, 'ir>( ir: &'ir IR<'n, 's>, ) -> Result, error::Error> { + let mut unique_number = UniqueNumber::new(); + match ir { IR::Query(ir) => { let mut query_plan = IndexMap::new(); for (alias, field) in ir { - query_plan.insert(alias.clone(), plan_query(field)?); + query_plan.insert(alias.clone(), plan_query(field, &mut unique_number)?); } Ok(RequestPlan::QueryPlan(query_plan)) } @@ -57,7 +59,7 @@ pub fn generate_request_plan<'n, 's, 'ir>( .insert(alias.clone(), type_name.clone()); } MutationRootField::ProcedureBasedCommand { selection_set, ir } => { - let plan = plan_mutation(selection_set, ir)?; + let plan = plan_mutation(selection_set, ir, &mut unique_number)?; mutation_plan .nodes .entry(plan.mutation_execution.data_connector.clone()) @@ -70,7 +72,7 @@ pub fn generate_request_plan<'n, 's, 'ir>( } IR::Subscription(alias, ir) => Ok(RequestPlan::SubscriptionPlan( alias.clone(), - plan_subscription(ir)?, + plan_subscription(ir, &mut unique_number)?, )), } } @@ -79,12 +81,13 @@ pub fn generate_request_plan<'n, 's, 'ir>( fn plan_mutation<'n, 's>( selection_set: &'n gql::normalized_ast::SelectionSet<'s, GDS>, ir: &ProcedureBasedCommand<'s>, + unique_number: &mut UniqueNumber, ) -> Result, error::Error> { let Plan { inner: ndc_ir, join_locations, remote_predicates, - } = commands::plan_mutation_execution(ir.procedure_name, ir)?; + } = commands::plan_mutation_execution(ir.procedure_name, ir, unique_number)?; // _should not_ happen but let's fail rather than do a query with missing filters if !remote_predicates.0.is_empty() { @@ -110,6 +113,7 @@ fn plan_mutation<'n, 's>( fn plan_subscription<'s, 'ir>( root_field: &'ir SubscriptionRootField<'_, 's>, + unique_number: &mut UniqueNumber, ) -> Result, error::Error> { match root_field { SubscriptionRootField::ModelSelectOne { @@ -117,7 +121,8 @@ fn plan_subscription<'s, 'ir>( selection_set, polling_interval_ms, } => { - let execution_tree = model_selection::plan_query_execution(&ir.model_selection)?; + let execution_tree = + model_selection::plan_query_execution(&ir.model_selection, unique_number)?; let query_execution_plan = reject_remote_joins(execution_tree)?; Ok(SubscriptionSelect { selection_set, @@ -138,7 +143,8 @@ fn plan_subscription<'s, 'ir>( selection_set, polling_interval_ms, } => { - let execution_tree = model_selection::plan_query_execution(&ir.model_selection)?; + let execution_tree = + model_selection::plan_query_execution(&ir.model_selection, unique_number)?; let query_execution_plan = reject_remote_joins(execution_tree)?; Ok(SubscriptionSelect { selection_set, @@ -159,7 +165,8 @@ fn plan_subscription<'s, 'ir>( selection_set, polling_interval_ms, } => { - let execution_tree = model_selection::plan_query_execution(&ir.model_selection)?; + let execution_tree = + model_selection::plan_query_execution(&ir.model_selection, unique_number)?; let query_execution_plan = reject_remote_joins(execution_tree)?; Ok(SubscriptionSelect { selection_set, @@ -185,6 +192,7 @@ fn reject_remote_joins(tree: ExecutionTree) -> Result( ir: &'ir QueryRootField<'n, 's>, + unique_number: &mut UniqueNumber, ) -> Result, error::Error> { let query_plan = match ir { QueryRootField::TypeName { type_name } => NodeQueryPlan::TypeName { @@ -211,7 +219,8 @@ fn plan_query<'n, 's, 'ir>( schema, }, QueryRootField::ModelSelectOne { ir, selection_set } => { - let execution_tree = model_selection::plan_query_execution(&ir.model_selection)?; + let execution_tree = + model_selection::plan_query_execution(&ir.model_selection, unique_number)?; NodeQueryPlan::NDCQueryExecution { selection_set, query_execution: NDCQueryExecution { @@ -226,7 +235,8 @@ fn plan_query<'n, 's, 'ir>( } QueryRootField::ModelSelectMany { ir, selection_set } => { - let execution_tree = model_selection::plan_query_execution(&ir.model_selection)?; + let execution_tree = + model_selection::plan_query_execution(&ir.model_selection, unique_number)?; NodeQueryPlan::NDCQueryExecution { selection_set, query_execution: NDCQueryExecution { @@ -240,7 +250,8 @@ fn plan_query<'n, 's, 'ir>( } } QueryRootField::ModelSelectAggregate { ir, selection_set } => { - let execution_tree = model_selection::plan_query_execution(&ir.model_selection)?; + let execution_tree = + model_selection::plan_query_execution(&ir.model_selection, unique_number)?; NodeQueryPlan::NDCQueryExecution { query_execution: NDCQueryExecution { execution_tree, @@ -253,7 +264,8 @@ fn plan_query<'n, 's, 'ir>( } QueryRootField::NodeSelect(optional_ir) => match optional_ir { Some(ir) => { - let execution_tree = model_selection::plan_query_execution(&ir.model_selection)?; + let execution_tree = + model_selection::plan_query_execution(&ir.model_selection, unique_number)?; NodeQueryPlan::RelayNodeSelect(Some(( NDCQueryExecution { execution_tree, @@ -267,7 +279,7 @@ fn plan_query<'n, 's, 'ir>( None => NodeQueryPlan::RelayNodeSelect(None), }, QueryRootField::FunctionBasedCommand { ir, selection_set } => { - let execution_tree = commands::plan_query_execution(ir)?; + let execution_tree = commands::plan_query_execution(ir, unique_number)?; NodeQueryPlan::NDCQueryExecution { selection_set, @@ -286,7 +298,8 @@ fn plan_query<'n, 's, 'ir>( QueryRootField::ApolloFederation(ApolloFederationRootFields::EntitiesSelect(irs)) => { let mut ndc_query_executions = Vec::new(); for ir in irs { - let execution_tree = model_selection::plan_query_execution(&ir.model_selection)?; + let execution_tree = + model_selection::plan_query_execution(&ir.model_selection, unique_number)?; ndc_query_executions.push(( NDCQueryExecution { execution_tree, diff --git a/v3/crates/graphql/ir/src/plan/arguments.rs b/v3/crates/graphql/ir/src/plan/arguments.rs index beac804f3cf..1cc9bb1d7b9 100644 --- a/v3/crates/graphql/ir/src/plan/arguments.rs +++ b/v3/crates/graphql/ir/src/plan/arguments.rs @@ -5,12 +5,14 @@ use std::collections::BTreeMap; use super::error as plan_error; use plan_types::{ Argument, MutationArgument, NdcRelationshipName, PredicateQueryTrees, Relationship, + UniqueNumber, }; /// Generate the argument plan from IR argument pub fn plan_argument( ir_argument: &crate::Argument<'_>, relationships: &mut BTreeMap, + unique_number: &mut UniqueNumber, ) -> Result<(Argument, PredicateQueryTrees), plan_error::Error> { let mut remote_predicates = PredicateQueryTrees::new(); let argument = match ir_argument { @@ -20,8 +22,12 @@ pub fn plan_argument( }) } crate::Argument::BooleanExpression { predicate } => { - let expression = - filter::plan_expression(predicate, relationships, &mut remote_predicates)?; + let expression = filter::plan_expression( + predicate, + relationships, + &mut remote_predicates, + unique_number, + )?; Ok(Argument::BooleanExpression { predicate: expression, @@ -37,6 +43,7 @@ pub fn plan_argument( pub fn plan_mutation_argument( ir_argument: &crate::Argument<'_>, relationships: &mut BTreeMap, + unique_number: &mut UniqueNumber, ) -> Result { let mut remote_predicates = PredicateQueryTrees::new(); @@ -47,8 +54,12 @@ pub fn plan_mutation_argument( }) } crate::Argument::BooleanExpression { predicate } => { - let expression = - super::filter::plan_expression(predicate, relationships, &mut remote_predicates)?; + let expression = super::filter::plan_expression( + predicate, + relationships, + &mut remote_predicates, + unique_number, + )?; Ok(MutationArgument::BooleanExpression { predicate: expression, @@ -62,6 +73,7 @@ pub fn plan_mutation_argument( pub fn plan_arguments( arguments: &BTreeMap>, relationships: &mut BTreeMap, + unique_number: &mut UniqueNumber, ) -> Result< ( BTreeMap, @@ -73,7 +85,8 @@ pub fn plan_arguments( let mut remote_predicates = PredicateQueryTrees::new(); for (argument_name, argument_value) in arguments { - let (argument, argument_remote_predicates) = plan_argument(argument_value, relationships)?; + let (argument, argument_remote_predicates) = + plan_argument(argument_value, relationships, unique_number)?; remote_predicates.0.extend(argument_remote_predicates.0); @@ -86,13 +99,14 @@ pub fn plan_arguments( pub fn plan_mutation_arguments( arguments: &BTreeMap>, relationships: &mut BTreeMap, + unique_number: &mut UniqueNumber, ) -> Result, plan_error::Error> { arguments .iter() .map(|(name, argument)| { Ok(( name.clone(), - plan_mutation_argument(argument, relationships)?, + plan_mutation_argument(argument, relationships, unique_number)?, )) }) .collect::, plan_error::Error>>() diff --git a/v3/crates/graphql/ir/src/plan/commands.rs b/v3/crates/graphql/ir/src/plan/commands.rs index 560be9b4c2c..acf0c5379ce 100644 --- a/v3/crates/graphql/ir/src/plan/commands.rs +++ b/v3/crates/graphql/ir/src/plan/commands.rs @@ -12,12 +12,13 @@ use open_dds::commands::ProcedureName; use plan_types::{ Argument, ExecutionTree, Field, FieldsSelection, JoinLocations, MutationExecutionPlan, NdcFieldAlias, NdcRelationshipName, PredicateQueryTrees, QueryExecutionPlan, QueryNodeNew, - Relationship, VariableName, FUNCTION_IR_VALUE_COLUMN_NAME, + Relationship, UniqueNumber, VariableName, FUNCTION_IR_VALUE_COLUMN_NAME, }; pub(crate) fn plan_query_node( ir: &CommandInfo<'_>, relationships: &mut BTreeMap, + unique_number: &mut UniqueNumber, ) -> Result, error::Error> { let mut ndc_nested_field = None; let mut join_locations = JoinLocations::new(); @@ -32,6 +33,7 @@ pub(crate) fn plan_query_node( nested_selection, ir.data_connector.capabilities.supported_ndc_version, relationships, + unique_number, )?; ndc_nested_field = Some(fields); join_locations = nested_join_locations; @@ -63,10 +65,14 @@ pub(crate) fn plan_query_node( pub(crate) fn plan_query_execution( ir: &FunctionBasedCommand<'_>, + unique_number: &mut UniqueNumber, ) -> Result { let mut collection_relationships = BTreeMap::new(); - let (mut arguments, mut remote_predicates) = - arguments::plan_arguments(&ir.command_info.arguments, &mut collection_relationships)?; + let (mut arguments, mut remote_predicates) = arguments::plan_arguments( + &ir.command_info.arguments, + &mut collection_relationships, + unique_number, + )?; // Add the variable arguments which are used for remote joins for (variable_name, variable_argument) in &ir.variable_arguments { @@ -82,7 +88,11 @@ pub(crate) fn plan_query_execution( inner: query_node, join_locations: remote_join_executions, remote_predicates: query_remote_predicates, - } = plan_query_node(&ir.command_info, &mut collection_relationships)?; + } = plan_query_node( + &ir.command_info, + &mut collection_relationships, + unique_number, + )?; remote_predicates.0.extend(query_remote_predicates.0); @@ -104,6 +114,7 @@ pub(crate) fn plan_query_execution( pub(crate) fn plan_mutation_execution( procedure_name: &ProcedureName, ir: &ProcedureBasedCommand<'_>, + unique_number: &mut UniqueNumber, ) -> Result, error::Error> { let mut ndc_nested_field = None; let mut join_locations = JoinLocations::new(); @@ -122,6 +133,7 @@ pub(crate) fn plan_mutation_execution( .capabilities .supported_ndc_version, &mut collection_relationships, + unique_number, )?; ndc_nested_field = Some(fields); join_locations = nested_join_locations; @@ -132,6 +144,7 @@ pub(crate) fn plan_mutation_execution( procedure_arguments: arguments::plan_mutation_arguments( &ir.command_info.arguments, &mut collection_relationships, + unique_number, )?, procedure_fields: ndc_nested_field, collection_relationships, diff --git a/v3/crates/graphql/ir/src/plan/filter.rs b/v3/crates/graphql/ir/src/plan/filter.rs index 273cf3abc97..ba4eb0991f3 100644 --- a/v3/crates/graphql/ir/src/plan/filter.rs +++ b/v3/crates/graphql/ir/src/plan/filter.rs @@ -7,7 +7,7 @@ use indexmap::IndexMap; use plan_types::{ ExecutionTree, Field, FieldsSelection, JoinLocations, NdcFieldAlias, NdcRelationshipName, PredicateQueryTree, PredicateQueryTrees, QueryExecutionPlan, QueryNodeNew, Relationship, - ResolvedFilterExpression, + ResolvedFilterExpression, UniqueNumber, }; /// Plan the filter expression IR. @@ -21,6 +21,7 @@ pub(crate) fn plan_filter_expression( relationship_join_filter, }: &FilterExpression<'_>, relationships: &mut BTreeMap, + unique_number: &mut UniqueNumber, ) -> Result<(Option, PredicateQueryTrees), plan_error::Error> { let mut expressions = Vec::new(); let mut remote_predicates = PredicateQueryTrees::new(); @@ -30,6 +31,7 @@ pub(crate) fn plan_filter_expression( filter, relationships, &mut remote_predicates, + unique_number, )?); } @@ -38,6 +40,7 @@ pub(crate) fn plan_filter_expression( filter, relationships, &mut remote_predicates, + unique_number, )?); } @@ -46,6 +49,7 @@ pub(crate) fn plan_filter_expression( filter, relationships, &mut remote_predicates, + unique_number, )?); } @@ -54,6 +58,7 @@ pub(crate) fn plan_filter_expression( query_where_expression, relationships, &mut remote_predicates, + unique_number, )?; expressions.push(planned_expression); } @@ -69,6 +74,7 @@ pub fn plan_expression<'a>( expression: &'a plan_types::Expression<'_>, relationships: &'a mut BTreeMap, remote_predicates: &'a mut PredicateQueryTrees, + unique_number: &mut UniqueNumber, ) -> Result { match expression { plan_types::Expression::And { @@ -76,7 +82,12 @@ pub fn plan_expression<'a>( } => { let mut results = Vec::new(); for and_expression in and_expressions { - let result = plan_expression(and_expression, relationships, remote_predicates)?; + let result = plan_expression( + and_expression, + relationships, + remote_predicates, + unique_number, + )?; results.push(result); } Ok(ResolvedFilterExpression::mk_and(results)) @@ -86,7 +97,12 @@ pub fn plan_expression<'a>( } => { let mut results = Vec::new(); for or_expression in or_expressions { - let result = plan_expression(or_expression, relationships, remote_predicates)?; + let result = plan_expression( + or_expression, + relationships, + remote_predicates, + unique_number, + )?; results.push(result); } Ok(ResolvedFilterExpression::mk_or(results)) @@ -94,7 +110,12 @@ pub fn plan_expression<'a>( plan_types::Expression::Not { expression: not_expression, } => { - let result = plan_expression(not_expression, relationships, remote_predicates)?; + let result = plan_expression( + not_expression, + relationships, + remote_predicates, + unique_number, + )?; Ok(ResolvedFilterExpression::mk_not(result)) } plan_types::Expression::LocalField(local_field_comparison) => Ok( @@ -105,7 +126,8 @@ pub fn plan_expression<'a>( field_path, column, } => { - let resolved_predicate = plan_expression(predicate, relationships, remote_predicates)?; + let resolved_predicate = + plan_expression(predicate, relationships, remote_predicates, unique_number)?; Ok(ResolvedFilterExpression::LocalNestedArray { column: column.clone(), field_path: field_path.clone(), @@ -117,7 +139,8 @@ pub fn plan_expression<'a>( predicate, info, } => { - let relationship_filter = plan_expression(predicate, relationships, remote_predicates)?; + let relationship_filter = + plan_expression(predicate, relationships, remote_predicates, unique_number)?; relationships.insert( relationship.clone(), process_model_relationship_definition(info)?, @@ -136,7 +159,7 @@ pub fn plan_expression<'a>( predicate, } => { let (remote_query_node, rest_predicate_trees, collection_relationships) = - plan_remote_predicate(ndc_column_mapping, predicate)?; + plan_remote_predicate(ndc_column_mapping, predicate, unique_number)?; let query_execution_plan: QueryExecutionPlan = QueryExecutionPlan { query_node: remote_query_node, @@ -157,7 +180,7 @@ pub fn plan_expression<'a>( children: rest_predicate_trees, }; - let remote_predicate_id = remote_predicates.insert(predicate_query_tree); + let remote_predicate_id = remote_predicates.insert(unique_number, predicate_query_tree); Ok(ResolvedFilterExpression::RemoteRelationshipComparison { remote_predicate_id, @@ -170,6 +193,7 @@ pub fn plan_expression<'a>( pub fn plan_remote_predicate<'a>( ndc_column_mapping: &'a [plan_types::RelationshipColumnMapping], predicate: &'a plan_types::Expression<'_>, + unique_number: &mut UniqueNumber, ) -> Result< ( QueryNodeNew, @@ -180,7 +204,12 @@ pub fn plan_remote_predicate<'a>( > { let mut relationships = BTreeMap::new(); let mut remote_predicates = PredicateQueryTrees::new(); - let planned_predicate = plan_expression(predicate, &mut relationships, &mut remote_predicates)?; + let planned_predicate = plan_expression( + predicate, + &mut relationships, + &mut remote_predicates, + unique_number, + )?; let query_node = QueryNodeNew { limit: None, diff --git a/v3/crates/graphql/ir/src/plan/model_selection.rs b/v3/crates/graphql/ir/src/plan/model_selection.rs index 52fa1573852..3ae8f965344 100644 --- a/v3/crates/graphql/ir/src/plan/model_selection.rs +++ b/v3/crates/graphql/ir/src/plan/model_selection.rs @@ -9,7 +9,7 @@ use crate::plan::Plan; use crate::ModelSelection; use plan_types::{ ExecutionTree, FieldsSelection, JoinLocations, NdcRelationshipName, PredicateQueryTrees, - QueryExecutionPlan, QueryNodeNew, Relationship, + QueryExecutionPlan, QueryNodeNew, Relationship, UniqueNumber, }; use std::collections::BTreeMap; @@ -18,6 +18,7 @@ use std::collections::BTreeMap; pub(crate) fn plan_query_node( ir: &ModelSelection<'_>, relationships: &mut BTreeMap, + unique_number: &mut UniqueNumber, ) -> Result, error::Error> { let mut query_fields = None; let mut join_locations = JoinLocations::new(); @@ -31,6 +32,7 @@ pub(crate) fn plan_query_node( selection, ir.data_connector.capabilities.supported_ndc_version, relationships, + unique_number, )?; query_fields = Some(fields); @@ -39,7 +41,7 @@ pub(crate) fn plan_query_node( } let (predicate, filter_remote_predicates) = - filter::plan_filter_expression(&ir.filter_clause, relationships)?; + filter::plan_filter_expression(&ir.filter_clause, relationships, unique_number)?; remote_predicates.0.extend(filter_remote_predicates.0); @@ -60,19 +62,22 @@ pub(crate) fn plan_query_node( } /// Generate query execution plan from internal IR (`ModelSelection`) -pub(crate) fn plan_query_execution(ir: &ModelSelection<'_>) -> Result { +pub(crate) fn plan_query_execution( + ir: &ModelSelection<'_>, + unique_number: &mut UniqueNumber, +) -> Result { let mut collection_relationships = BTreeMap::new(); let Plan { inner: query, join_locations: remote_join_executions, mut remote_predicates, - } = plan_query_node(ir, &mut collection_relationships)?; + } = plan_query_node(ir, &mut collection_relationships, unique_number)?; // collection relationships from order_by clause relationships::collect_relationships_from_order_by(ir, &mut collection_relationships)?; let (arguments, argument_remote_predicates) = - arguments::plan_arguments(&ir.arguments, &mut collection_relationships)?; + arguments::plan_arguments(&ir.arguments, &mut collection_relationships, unique_number)?; remote_predicates.0.extend(argument_remote_predicates.0); diff --git a/v3/crates/graphql/ir/src/plan/selection_set.rs b/v3/crates/graphql/ir/src/plan/selection_set.rs index 3b95df996fe..cb693ab69c4 100644 --- a/v3/crates/graphql/ir/src/plan/selection_set.rs +++ b/v3/crates/graphql/ir/src/plan/selection_set.rs @@ -15,13 +15,14 @@ use plan_types::{ NestedField, NestedObject, PredicateQueryTrees, RemoteJoin, RemoteJoinType, SourceFieldAlias, TargetField, }; -use plan_types::{NdcFieldAlias, NdcRelationshipName, Relationship}; +use plan_types::{NdcFieldAlias, NdcRelationshipName, Relationship, UniqueNumber}; use std::collections::{BTreeMap, HashMap}; pub(crate) fn plan_nested_selection( nested_selection: &NestedSelection<'_>, ndc_version: NdcVersion, relationships: &mut BTreeMap, + unique_number: &mut UniqueNumber, ) -> Result, error::Error> { match nested_selection { NestedSelection::Object(model_selection) => { @@ -29,7 +30,7 @@ pub(crate) fn plan_nested_selection( inner: fields, join_locations, remote_predicates, - } = plan_selection_set(model_selection, ndc_version, relationships)?; + } = plan_selection_set(model_selection, ndc_version, relationships, unique_number)?; Ok(Plan { inner: NestedField::Object(NestedObject { fields }), join_locations, @@ -41,7 +42,7 @@ pub(crate) fn plan_nested_selection( inner: field, join_locations, remote_predicates, - } = plan_nested_selection(nested_selection, ndc_version, relationships)?; + } = plan_nested_selection(nested_selection, ndc_version, relationships, unique_number)?; Ok(Plan { inner: NestedField::Array(NestedArray { fields: Box::new(field), @@ -62,6 +63,7 @@ pub(crate) fn plan_selection_set( model_selection: &ResultSelectionSet<'_>, ndc_version: NdcVersion, relationships: &mut BTreeMap, + unique_number: &mut UniqueNumber, ) -> Result>, error::Error> { let mut fields = IndexMap::::new(); let mut join_locations = JoinLocations::new(); @@ -80,7 +82,12 @@ pub(crate) fn plan_selection_set( inner: nested_fields, join_locations: jl, remote_predicates: nested_remote_predicates, - } = plan_nested_selection(nested_selection, ndc_version, relationships)?; + } = plan_nested_selection( + nested_selection, + ndc_version, + relationships, + unique_number, + )?; remote_predicates.0.extend(nested_remote_predicates.0); @@ -88,7 +95,7 @@ pub(crate) fn plan_selection_set( nested_join_locations = jl; } let (arguments, argument_remote_predicates) = - arguments::plan_arguments(arguments, relationships)?; + arguments::plan_arguments(arguments, relationships, unique_number)?; remote_predicates.0.extend(argument_remote_predicates.0); @@ -124,7 +131,7 @@ pub(crate) fn plan_selection_set( inner: relationship_query, join_locations: jl, remote_predicates: relationship_remote_predicates, - } = model_selection::plan_query_node(query, relationships)?; + } = model_selection::plan_query_node(query, relationships, unique_number)?; remote_predicates.0.extend(relationship_remote_predicates.0); @@ -158,12 +165,16 @@ pub(crate) fn plan_selection_set( inner: relationship_query, join_locations: jl, remote_predicates: command_remote_predicates, - } = commands::plan_query_node(&ir.command_info, relationships)?; + } = commands::plan_query_node(&ir.command_info, relationships, unique_number)?; remote_predicates.0.extend(command_remote_predicates.0); let (relationship_arguments, argument_remote_predicates) = - arguments::plan_arguments(&ir.command_info.arguments, relationships)?; + arguments::plan_arguments( + &ir.command_info.arguments, + relationships, + unique_number, + )?; remote_predicates.0.extend(argument_remote_predicates.0); @@ -209,7 +220,7 @@ pub(crate) fn plan_selection_set( query_execution_plan: query_execution, remote_join_executions: sub_join_locations, remote_predicates: model_remote_predicates, - } = model_selection::plan_query_execution(ir)?; + } = model_selection::plan_query_execution(ir, unique_number)?; // push any remote predicates to the outer list remote_predicates.0.extend(model_remote_predicates.0); @@ -255,7 +266,7 @@ pub(crate) fn plan_selection_set( query_execution_plan: ndc_ir, remote_join_executions: sub_join_locations, remote_predicates: command_remote_predicates, - } = commands::plan_query_execution(ir)?; + } = commands::plan_query_execution(ir, unique_number)?; remote_predicates.0.extend(command_remote_predicates.0); diff --git a/v3/crates/plan-types/Cargo.toml b/v3/crates/plan-types/Cargo.toml index fb61d5514d1..5d809858b50 100644 --- a/v3/crates/plan-types/Cargo.toml +++ b/v3/crates/plan-types/Cargo.toml @@ -20,7 +20,6 @@ schemars = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } smol_str = { workspace = true } -uuid = { workspace = true } [lints] workspace = true diff --git a/v3/crates/plan-types/src/execution_plan.rs b/v3/crates/plan-types/src/execution_plan.rs index 9fe54dbec93..22f1b5486dc 100644 --- a/v3/crates/plan-types/src/execution_plan.rs +++ b/v3/crates/plan-types/src/execution_plan.rs @@ -19,6 +19,7 @@ pub use mutation::MutationExecutionPlan; pub use order_by::{OrderByDirection, OrderByElement, OrderByTarget}; pub use query::{ FieldsSelection, PredicateQueryTree, PredicateQueryTrees, QueryExecutionPlan, QueryNodeNew, + RemotePredicateKey, UniqueNumber, }; pub use relationships::{Relationship, RelationshipArgument}; pub use remote_joins::{ diff --git a/v3/crates/plan-types/src/execution_plan/filter.rs b/v3/crates/plan-types/src/execution_plan/filter.rs index 6de7949864c..7ced748910a 100644 --- a/v3/crates/plan-types/src/execution_plan/filter.rs +++ b/v3/crates/plan-types/src/execution_plan/filter.rs @@ -1,6 +1,5 @@ -use crate::{LocalFieldComparison, NdcRelationshipName}; +use crate::{LocalFieldComparison, NdcRelationshipName, RemotePredicateKey}; use open_dds::data_connector::DataConnectorColumnName; -use uuid::Uuid; /// Filter expression plan to be resolved #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -25,7 +24,7 @@ pub enum ResolvedFilterExpression { predicate: Box, }, RemoteRelationshipComparison { - remote_predicate_id: Uuid, + remote_predicate_id: RemotePredicateKey, }, } diff --git a/v3/crates/plan-types/src/execution_plan/query.rs b/v3/crates/plan-types/src/execution_plan/query.rs index ea98b8303bc..4cc5b957f54 100644 --- a/v3/crates/plan-types/src/execution_plan/query.rs +++ b/v3/crates/plan-types/src/execution_plan/query.rs @@ -5,7 +5,6 @@ use indexmap::IndexMap; use open_dds::{data_connector::CollectionName, types::DataConnectorArgumentName}; use std::collections::BTreeMap; use std::sync::Arc; -use uuid::Uuid; #[derive(Debug, Clone, PartialEq)] // this represents an execution plan. all predicates only refer to local comparisons. @@ -34,14 +33,18 @@ pub struct PredicateQueryTree { } #[derive(Debug, Clone, PartialEq)] -pub struct PredicateQueryTrees(pub BTreeMap); +pub struct PredicateQueryTrees(pub BTreeMap); impl PredicateQueryTrees { pub fn new() -> Self { Self(BTreeMap::new()) } - pub fn insert(&mut self, value: PredicateQueryTree) -> Uuid { - let key = Uuid::new_v4(); + pub fn insert( + &mut self, + unique_number: &mut UniqueNumber, + value: PredicateQueryTree, + ) -> RemotePredicateKey { + let key = RemotePredicateKey(unique_number.fresh()); self.0.insert(key, value); key } @@ -53,6 +56,32 @@ impl Default for PredicateQueryTrees { } } +#[derive(Debug, PartialEq, Eq, PartialOrd, derive_more::Display, Ord, Hash, Clone, Copy)] +pub struct RemotePredicateKey(pub u64); + +// we need to generate unique identifiers for remote predicates +// in a reproducable fashion so we thread this around +pub struct UniqueNumber(u64); + +impl UniqueNumber { + pub fn new() -> Self { + UniqueNumber(1) + } + + // get the next number, increment internal value + pub fn fresh(&mut self) -> u64 { + let value = self.0; + self.0 += 1; + value + } +} + +impl Default for UniqueNumber { + fn default() -> Self { + Self::new() + } +} + /// Query plan for fetching data #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct QueryNodeNew { diff --git a/v3/crates/plan-types/src/lib.rs b/v3/crates/plan-types/src/lib.rs index 2467c0aa8d7..8bd7172add2 100644 --- a/v3/crates/plan-types/src/lib.rs +++ b/v3/crates/plan-types/src/lib.rs @@ -14,7 +14,7 @@ pub use execution_plan::{ NestedArray, NestedField, NestedObject, OrderByDirection, OrderByElement, OrderByTarget, PredicateQueryTree, PredicateQueryTrees, ProcessResponseAs, QueryExecutionPlan, QueryNodeNew, Relationship, RelationshipArgument, RemoteJoin, RemoteJoinArgument, RemoteJoinType, - ResolvedFilterExpression, SourceFieldAlias, TargetField, + RemotePredicateKey, ResolvedFilterExpression, SourceFieldAlias, TargetField, UniqueNumber, }; pub use expression::{ ComparisonTarget, ComparisonValue, Expression, LocalFieldComparison, RelationshipColumnMapping,