This issue is meant to track work on an initiative and will have related issues.
At a high level, let's prune the data structure to precisely what is necessary to execute a query plan.
Currently, for a federated graph, Gateway maintains a number of graphql AST nodes (as in, nodes which were parsed with graphql.parse) on its data structure that represents the query plan.
Originally, these AST nodes:
GraphQLResponse's extensions property, when enabled.)However, as we mull about a representation of the query plan that is more portable (and more testable, across different implementations), these AST nodes exist as parser-specific implementation details.
Some of the properties are absolutely still relevant, e.g., kind, name, alias (more?). Instead of merely attaching the AST nodes, we should determine a stable structure for the query plan which can represent these important values and let go of unimportant bits (e.g., location). We can probably put all of the relevant values into their own structure.
For starters, we can use existing service definition fixtures that live within our tests (i.e. products, or reviews) to generate a query plan, put it in this issue and discuss.
After chatting with @jbaxleyiii about this, I think we've roughly figured out what is required, what can change, and what can go.
A list of changes:
stringNamedTypeNode to stringThe proposed TS types would look like:
export interface QueryPlan {
kind: 'QueryPlan';
node?: PlanNode;
}
export type PlanNode = SequenceNode | ParallelNode | FetchNode | FlattenNode;
export interface SequenceNode {
kind: 'Sequence';
nodes: PlanNode[];
}
export interface ParallelNode {
kind: 'Parallel';
nodes: PlanNode[];
}
export interface FetchNode {
kind: 'Fetch';
serviceName: string;
variableUsages?: string[];
requires?: SelectionNode[];
source: string;
}
export interface FlattenNode {
kind: 'Flatten';
path: ResponsePath;
node: PlanNode;
}
export type SelectionNode = FieldNode | InlineFragmentNode;
export interface FieldNode {
readonly kind: 'Field';
readonly alias?: string;
readonly name: string;
readonly selections?: SelectionNode[];
}
export interface InlineFragmentNode {
readonly kind: 'InlineFragment';
readonly typeCondition?: string;
readonly selections: SelectionNode[];
}
diff --git a/plan.json b/plan.json
index a0293b6..813223a 100644
--- a/plan.json
+++ b/plan.json
@@ -6,243 +6,43 @@
{
"kind": "Fetch",
"serviceName": "books",
- "selectionSet": {
- "kind": "SelectionSet",
- "selections": [
- {
- "kind": "Field",
- "name": {
- "kind": "Name",
- "value": "books"
- },
- "arguments": [],
- "directives": [],
- "selectionSet": {
- "kind": "SelectionSet",
- "selections": [
- {
- "kind": "Field",
- "name": {
- "kind": "Name",
- "value": "__typename"
- }
- },
- {
- "kind": "Field",
- "name": {
- "kind": "Name",
- "value": "isbn",
- "loc": {
- "start": 8,
- "end": 12
- }
- },
- "arguments": [],
- "directives": [],
- "loc": {
- "start": 8,
- "end": 12
- }
- },
- {
- "kind": "Field",
- "name": {
- "kind": "Name",
- "value": "title",
- "loc": {
- "start": 8,
- "end": 13
- }
- },
- "arguments": [],
- "directives": [],
- "loc": {
- "start": 8,
- "end": 13
- }
- },
- {
- "kind": "Field",
- "name": {
- "kind": "Name",
- "value": "year",
- "loc": {
- "start": 14,
- "end": 18
- }
- },
- "arguments": [],
- "directives": [],
- "loc": {
- "start": 14,
- "end": 18
- }
- }
- ]
- }
- }
- ]
- },
- "variableUsages": {},
- "internalFragments": {},
+ "variableUsages": [],
"source": "{books{__typename isbn title year}}"
},
{
"kind": "Flatten",
- "path": [
- "books",
- "@"
- ],
+ "path": ["books", "@"],
"node": {
"kind": "Fetch",
"serviceName": "product",
- "selectionSet": {
- "kind": "SelectionSet",
- "selections": [
- {
- "kind": "InlineFragment",
- "typeCondition": {
- "kind": "NamedType",
- "name": {
- "kind": "Name",
- "value": "Book"
- }
+ "requires": [
+ {
+ "kind": "InlineFragment",
+ "typeCondition": "Book",
+ "selections": [
+ {
+ "kind": "Field",
+ "name": "__typename"
},
- "selectionSet": {
- "kind": "SelectionSet",
- "selections": [
- {
- "kind": "Field",
- "name": {
- "kind": "Name",
- "value": "name"
- },
- "arguments": [
- {
- "kind": "Argument",
- "name": {
- "kind": "Name",
- "value": "delimiter"
- },
- "value": {
- "kind": "Variable",
- "name": {
- "kind": "Name",
- "value": "delimiter"
- }
- }
- }
- ],
- "directives": []
- }
- ]
- }
- }
- ]
- },
- "requires": {
- "kind": "SelectionSet",
- "selections": [
- {
- "kind": "InlineFragment",
- "typeCondition": {
- "kind": "NamedType",
- "name": {
- "kind": "Name",
- "value": "Book"
- }
+ {
+ "kind": "Field",
+ "name": "isbn"
},
- "selectionSet": {
- "kind": "SelectionSet",
- "selections": [
- {
- "kind": "Field",
- "name": {
- "kind": "Name",
- "value": "__typename"
- }
- },
- {
- "kind": "Field",
- "name": {
- "kind": "Name",
- "value": "isbn",
- "loc": {
- "start": 8,
- "end": 12
- }
- },
- "arguments": [],
- "directives": [],
- "loc": {
- "start": 8,
- "end": 12
- }
- },
- {
- "kind": "Field",
- "name": {
- "kind": "Name",
- "value": "title",
- "loc": {
- "start": 8,
- "end": 13
- }
- },
- "arguments": [],
- "directives": [],
- "loc": {
- "start": 8,
- "end": 13
- }
- },
- {
- "kind": "Field",
- "name": {
- "kind": "Name",
- "value": "year",
- "loc": {
- "start": 14,
- "end": 18
- }
- },
- "arguments": [],
- "directives": [],
- "loc": {
- "start": 14,
- "end": 18
- }
- }
- ]
- }
- }
- ]
- },
- "variableUsages": {
- "delimiter": {
- "kind": "VariableDefinition",
- "variable": {
- "kind": "Variable",
- "name": {
- "kind": "Name",
- "value": "delimiter"
- }
- },
- "type": {
- "kind": "NamedType",
- "name": {
- "kind": "Name",
- "value": "String"
+ {
+ "kind": "Field",
+ "name": "title"
+ },
+ {
+ "kind": "Field",
+ "name": "year"
}
- },
- "directives": []
+ ]
}
- },
- "internalFragments": {},
+ ],
+ "variableUsages": ["delimiter"],
"source": "query($representations:[_Any!]!$delimiter:String){_entities(representations:$representations){...on Book{name(delimiter:$delimiter)}}}"
}
}
]
}
}
-
If this is helpful, I went through this process on the Rust side and ended up with
pub struct QueryPlan(pub Option<PlanNode>);
pub enum PlanNode {
Sequence(Vec<PlanNode>),
Parallel(Vec<PlanNode>),
Fetch(FetchNode),
Flatten {
path: Vec<ResponsePathElement>,
node: Box<PlanNode>,
},
}
pub struct FetchNode {
pub service_name: String,
pub variable_usages: Vec<String>,
pub requires: Option<SelectionSet>,
pub operation: GraphQLDocument,
}
pub enum Selection {
Field(Field),
InlineFragment(InlineFragment),
}
pub struct Field {
pub alias: Option<String>,
pub name: String,
pub selection_set: SelectionSet,
}
pub struct InlineFragment {
pub type_condition: Option<String>,
pub selection_set: SelectionSet,
}
pub enum ResponsePathElement {
Field(String),
Idx(u32),
}
pub type SelectionSet = Vec<Selection>;
pub type GraphQLDocument = String;
The shape looks right to me now, particularly with requires being an array of selection sets rather than _an object with a selections array of selection sets_.
While I realize the inspiration is entirely shared, do we think the presence of Node in the new type names (e.g., SelectionSetNode, InlineFragmentNode) evokes too much of a relationship to the identically-named AST _nodes_? They are structurally identical, but not the same types (i.e., not precisely those from graphql-js), right?
I just want to make sure we're not inadvertently creating a confusing pattern.
Anyhow, I think this LGTM (whatever the stamp-of-approval for _issues_ is ✅ ) and could move toward implementation changes. We can certainly adjust for naming later, as we fancy it, or as we note in the test snapshot updates that will eventually surface from here. 😄
(hmm for Rust, hmm the type of requires is Option<Vec<Selection>> but I suppose we can go with Vec<Selection> and an empty vector is the same as a None)
@Enrico2 Isn't that equivalent to what we have?
Option<Vec<Selection>>
requires?: SelectionNode[];
oh yeah I didn't see the ? -- the extra Option there seems redundant? It'll lead to easier code if we default to an empty array/vector?
@JakeDawkins oh I guess it prints nicer by just not including the field all together. No strong opinion there.
How do you feel about renaming it to operation instead of source ?
@JakeDawkins I just noticed this: https://rustwasm.github.io/docs/wasm-bindgen/reference/types/exported-rust-types.html
It seems that wasm bridging doesn't support Option -- I'll see if this is an issue as I explore implementing the bridge. If this is an issue and we want parity, then we might have to refactor the node side a little.
The reason I think we _may_ want parity is that we could use a response coming from rust directly in ts, but at the same time I'm not sure being tied to it is a good idea..? idk. Anyway, as I learn about the wasm bridge more, I'll update.
@JakeDawkins oh I guess it prints nicer by just not including the field all together. No strong opinion there.
How do you feel about renaming it to
operationinstead ofsource?
I think that's pretty reasonable, and probably more descriptive of what actually goes in that field
Related to #4410 #4448
Most helpful comment
If this is helpful, I went through this process on the Rust side and ended up with