Conversation
the query plan only depends on the query, operation name and query plan options, so any chages by plugins should be deterministic. Working from that assumption, the query planner cache should be able to store the result of plugins and query planner execution (instead of applying plugins everytime as it is done currently)
This comment has been minimized.
This comment has been minimized.
|
How does this new caching interact with the mutability of |
Context is still used to set the usage reporting field for telemetry so it should stay. When we retrieve the cached response, the context for the current request is updated accordingly |
SimonSapin
left a comment
SimonSapin
left a comment
There was a problem hiding this comment.
I think it’d be good to document for plugin authors that their Plugin::query_planning_service may be cached, what’s the cache key, and how they can influence it (from a router/supergraph service)
good idea |
| inner_e | ||
| ); | ||
| Err(error) => { | ||
| if let Some(error) = error.downcast_ref::<QueryPlannerError>() { |
There was a problem hiding this comment.
Just a question, did you already test it ? I sometimes played with that and sometimes it doesn't work because you need to fetch the error.source()
There was a problem hiding this comment.
if a plugin replaces the error then that will not work, but there won't be anything we can do about it
| retrieval_error.deref(), | ||
| QueryPlannerError::SpecError(_) | ||
| | QueryPlannerError::SchemaValidationErrors(_) | ||
| retrieval_error.deref().downcast_ref::<QueryPlannerError>(), |
There was a problem hiding this comment.
same comment about .source() and so one
3 participants
Fix #1412
the query plan only depends on the query, operation name and query plan
options, so any chages by plugins should be deterministic.
Working from that assumption, the query planner cache should be able to
store the result of plugins and query planner execution (instead of
applying plugins everytime as it is done currently)