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

chore(federation): update outdated comments in QueryPlannerConfig #6229

Merged
merged 2 commits into from
Nov 5, 2024
Merged
Changes from all commits
Commits
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
34 changes: 16 additions & 18 deletions apollo-federation/src/query_plan/query_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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(),
Expand All @@ -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,
}

Expand Down
Loading