From f9ba710d6bb29dffca7dbeb68ab5db95a5e639e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Tue, 5 Nov 2024 10:06:04 +0100 Subject: [PATCH 1/2] chore(federation): update outdated comments in QueryPlannerConfig --- .../src/query_plan/query_planner.rs | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/apollo-federation/src/query_plan/query_planner.rs b/apollo-federation/src/query_plan/query_planner.rs index 24e0972944..7a8d89b0cc 100644 --- a/apollo-federation/src/query_plan/query_planner.rs +++ b/apollo-federation/src/query_plan/query_planner.rs @@ -56,31 +56,26 @@ pub(crate) const CONTEXT_DIRECTIVE: &str = "context"; #[derive(Debug, Clone, Hash)] pub struct QueryPlannerConfig { - /// Whether the query planner should try to reused the named fragments of the planned query in + /// Whether the query planner should try to reuse the named fragments of the planned query in /// subgraph fetches. /// - /// This is often a good idea as it can prevent very large subgraph queries in some cases (named - /// fragments can make some relatively small queries (using said fragments) expand to a very large - /// query if all the spreads are inline). However, due to architecture of the query planner, this - /// optimization is done as an additional pass on the subgraph queries of the generated plan and - /// can thus increase the latency of building a plan. As long as query plans are sufficiently - /// cached, this should not be a problem, which is why this option is enabled by default, but if - /// the distribution of inbound queries prevents efficient caching of query plans, this may become - /// an undesirable trade-off and can be disabled in that case. + /// Reusing fragments requires complicated validations, so it can take a long time on large + /// queries with many fragments. This option may be removed in the future in favour of + /// [`generate_query_fragments`][QueryPlannerConfig::generate_query_fragments]. /// /// Defaults to true. pub reuse_query_fragments: bool, - /// NOTE: **not implemented yet** - /// /// If enabled, the query planner will extract inline fragments into fragment /// definitions before sending queries to subgraphs. This can significantly - /// reduce the size of the query sent to subgraphs, but may increase the time - /// it takes to plan the query. + /// reduce the size of the query sent to subgraphs. /// /// Defaults to false. pub generate_query_fragments: bool, + /// **TODO:** This option is not implemented, and the behaviour is *always enabled*. + /// https://github.com/apollographql/router/pull/5871 + /// /// Whether to run GraphQL validation against the extracted subgraph schemas. Recommended in /// non-production settings or when debugging. /// @@ -112,8 +107,8 @@ impl Default for QueryPlannerConfig { fn default() -> Self { Self { reuse_query_fragments: true, - subgraph_graphql_validation: false, generate_query_fragments: false, + subgraph_graphql_validation: false, incremental_delivery: Default::default(), debug: Default::default(), type_conditioned_fetching: Default::default(), @@ -123,12 +118,15 @@ impl Default for QueryPlannerConfig { #[derive(Debug, Clone, Default, Hash)] pub struct QueryPlanIncrementalDeliveryConfig { - /// Enables @defer support by the query planner. + /// Enables `@defer` support in the query planner, breaking up the query plan with [DeferNode]s + /// as appropriate. + /// + /// If false, operations with `@defer` are still accepted, but are planned as if they did not + /// contain `@defer` directives. /// - /// If set, then the query plan for queries having some @defer will contains some `DeferNode` - /// (see `query_plan/mod.rs`). + /// Defaults to false. /// - /// Defaults to false (meaning that the @defer are ignored). + /// [DeferNode]: crate::query_plan::DeferNode pub enable_defer: bool, } From 7fbd57631063d0a40a0db6d54968e0ea47e7ba50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Tue, 5 Nov 2024 10:38:51 +0100 Subject: [PATCH 2/2] lint --- apollo-federation/src/query_plan/query_planner.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apollo-federation/src/query_plan/query_planner.rs b/apollo-federation/src/query_plan/query_planner.rs index 7a8d89b0cc..1a3f1002f1 100644 --- a/apollo-federation/src/query_plan/query_planner.rs +++ b/apollo-federation/src/query_plan/query_planner.rs @@ -74,7 +74,7 @@ pub struct QueryPlannerConfig { pub generate_query_fragments: bool, /// **TODO:** This option is not implemented, and the behaviour is *always enabled*. - /// https://github.com/apollographql/router/pull/5871 + /// /// /// Whether to run GraphQL validation against the extracted subgraph schemas. Recommended in /// non-production settings or when debugging.