Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(federation): query plan correctness checker #6498

Merged
merged 29 commits into from
Feb 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
18f1a29
feat(federation): started query plan correctness checker
duckki Dec 13, 2024
5eab255
implemented basic correctness check of query plans
duckki Jan 3, 2025
85aae0f
refactor: handle interpret_query_plan failures as mismatch error
duckki Jan 13, 2025
7c41d20
debug: added more fmt::Display implementations for response shape-rel…
duckki Jan 14, 2025
b1e0f27
debug: improved response shape display to suppress showing type condi…
duckki Jan 15, 2025
ed0d303
fix: merge subscription's rest node from the state after the primary
duckki Jan 15, 2025
7a9b39b
fix: support type-conditioned fetching
duckki Jan 14, 2025
f33f9ae
fix: infer subgraph constraints
duckki Jan 15, 2025
acab67e
refactor: use the set-intersection notation (∩) for type conditions
duckki Jan 19, 2025
23ec404
refactor: moved the correctness module to apollo-federation/src/
duckki Jan 28, 2025
b8217a6
added `compare_operations` function
duckki Jan 28, 2025
fdb9a02
fix: check if entity type definitions are resolvable
duckki Jan 18, 2025
fb9e5d2
refactor: cleaning up
duckki Jan 28, 2025
294c280
added an extra sanity check where multiple typename conditions are pr…
duckki Jan 29, 2025
cb532f9
fix: use both api schema and supergraph schema for query plan check
duckki Jan 29, 2025
fb4c1cd
added support for Defer nodes
duckki Jan 29, 2025
ce0fdf0
cleaned up FetchDataPathElement handling
duckki Jan 30, 2025
c917d5b
added few tracing::debug outputs in check_plan function
duckki Jan 30, 2025
df76d21
fix: fixed a gap in compare_possible_definitions
duckki Jan 31, 2025
00c6772
refactor: moved `coerce_executable_values` call to `check_plan` funct…
duckki Feb 8, 2025
d331e39
refactor: feature-gated the `correctness` module
duckki Feb 8, 2025
57e065b
refactor: simplified iterator usages and lifetime parameters
duckki Feb 8, 2025
ce362c2
refactor: revamped the correctness error types
duckki Feb 8, 2025
5a1e158
refactor: another iterator clean-up
duckki Feb 8, 2025
6f296be
refactor: removed the prematurely included soundness check
duckki Feb 8, 2025
06fa4c4
refactor: adjusted debug tracing output for the public correctness fu…
duckki Feb 8, 2025
a56f8d9
added a few comments
duckki Feb 8, 2025
8d033a0
cargo xtask lint
duckki Feb 8, 2025
4cc80de
Merge branch 'dev' into duckki/correctness
duckki Feb 8, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apollo-federation/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ autotests = false # Integration tests are m
# This logging is gated behind a feature to avoid any unnecessary (even if
# small) runtime costs where this data will not be desired.
snapshot_tracing = ["ron"]
# `correctness` feature enables the `correctness` module.
correctness = []

[dependencies]
apollo-compiler.workspace = true
Expand Down
2 changes: 1 addition & 1 deletion apollo-federation/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ edition = "2021"

[dependencies]
apollo-compiler.workspace = true
apollo-federation = { path = ".." }
apollo-federation = { path = "..", features = ["correctness"] }
clap = { version = "4.5.1", features = ["derive"] }
tracing-subscriber = { version = "0.3", features = ["env-filter"] }

Expand Down
30 changes: 26 additions & 4 deletions apollo-federation/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ use std::path::PathBuf;
use std::process::ExitCode;

use apollo_compiler::ExecutableDocument;
use apollo_federation::correctness::CorrectnessError;
use apollo_federation::error::FederationError;
use apollo_federation::error::SingleFederationError;
use apollo_federation::internal_error;
use apollo_federation::query_graph;
use apollo_federation::query_plan::query_planner::QueryPlanner;
use apollo_federation::query_plan::query_planner::QueryPlannerConfig;
Expand Down Expand Up @@ -288,11 +290,31 @@ fn cmd_plan(

let query_doc =
ExecutableDocument::parse_and_validate(planner.api_schema().schema(), query, query_path)?;
print!(
"{}",
planner.build_query_plan(&query_doc, None, Default::default())?
let query_plan = planner.build_query_plan(&query_doc, None, Default::default())?;
println!("{query_plan}");

// Check the query plan
let subgraphs_by_name = supergraph
.extract_subgraphs()
.unwrap()
.into_iter()
.map(|(name, subgraph)| (name, subgraph.schema))
.collect();
let result = apollo_federation::correctness::check_plan(
planner.api_schema(),
&supergraph.schema,
&subgraphs_by_name,
&query_doc,
&query_plan,
);
Ok(())
match result {
Ok(_) => Ok(()),
Err(CorrectnessError::FederationError(e)) => Err(e),
Err(CorrectnessError::ComparisonError(e)) => Err(internal_error!(
"Response shape from query plan does not match response shape from input operation:\n{}",
e.description()
)),
}
}

fn cmd_validate(file_paths: &[PathBuf]) -> Result<(), FederationError> {
Expand Down
83 changes: 83 additions & 0 deletions apollo-federation/src/correctness/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
pub mod query_plan_analysis;
#[cfg(test)]
pub mod query_plan_analysis_test;
pub mod response_shape;
pub mod response_shape_compare;
#[cfg(test)]
pub mod response_shape_test;
mod subgraph_constraint;

use std::sync::Arc;

use apollo_compiler::collections::IndexMap;
use apollo_compiler::validation::Valid;
use apollo_compiler::ExecutableDocument;

use crate::compat::coerce_executable_values;
use crate::correctness::response_shape_compare::compare_response_shapes_with_constraint;
use crate::correctness::response_shape_compare::ComparisonError;
use crate::query_plan::QueryPlan;
use crate::schema::ValidFederationSchema;
use crate::FederationError;

//==================================================================================================
// Public API

#[derive(derive_more::From)]
pub enum CorrectnessError {
/// Correctness checker's own error
FederationError(FederationError),
/// Error in the input that is subject to comparison
ComparisonError(ComparisonError),
}

/// Check if `this`'s response shape is a subset of `other`'s response shape.
pub fn compare_operations(
schema: &ValidFederationSchema,
this: &Valid<ExecutableDocument>,
other: &Valid<ExecutableDocument>,
) -> Result<(), CorrectnessError> {
let this_rs = response_shape::compute_response_shape_for_operation(this, schema)?;
let other_rs = response_shape::compute_response_shape_for_operation(other, schema)?;
tracing::debug!(
"compare_operations:\nResponse shape (left): {this_rs}\nResponse shape (right): {other_rs}"
);
Ok(response_shape_compare::compare_response_shapes(
&this_rs, &other_rs,
)?)
}

/// Check the correctness of the query plan against the schema and input operation by comparing
/// the response shape of the input operation and the response shape of the query plan.
/// - The input operation's response shape is supposed to be a subset of the input operation's.
pub fn check_plan(
api_schema: &ValidFederationSchema,
supergraph_schema: &ValidFederationSchema,
subgraphs_by_name: &IndexMap<Arc<str>, ValidFederationSchema>,
operation_doc: &Valid<ExecutableDocument>,
plan: &QueryPlan,
) -> Result<(), CorrectnessError> {
// Coerce constant expressions in the input operation document since query planner does it for
// subgraph fetch operations. But, this may be unnecessary in the future (see ROUTER-816).
let mut operation_doc = operation_doc.clone().into_inner();
coerce_executable_values(api_schema.schema(), &mut operation_doc);
let operation_doc = operation_doc
.validate(api_schema.schema())
.map_err(FederationError::from)?;

let op_rs = response_shape::compute_response_shape_for_operation(&operation_doc, api_schema)?;
let root_type = response_shape::compute_the_root_type_condition_for_operation(&operation_doc)?;
let plan_rs = query_plan_analysis::interpret_query_plan(supergraph_schema, &root_type, plan)
.map_err(|e| {
ComparisonError::new(format!(
"Failed to compute the response shape from query plan:\n{e}"
))
})?;
tracing::debug!(
"check_plan:\nOperation response shape: {op_rs}\nQuery plan response shape: {plan_rs}"
);

let path_constraint = subgraph_constraint::SubgraphConstraint::at_root(subgraphs_by_name);
compare_response_shapes_with_constraint(&path_constraint, &op_rs, &plan_rs)?;
Ok(())
}
Loading