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

Add new generateQueryFragments option to query planner config #2958

Merged
merged 18 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from 8 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
9 changes: 9 additions & 0 deletions .changeset/spicy-falcons-learn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"apollo-federation-integration-testsuite": minor
"@apollo/query-planner": minor
"@apollo/federation-internals": minor
---

Add new `generateQueryFragments` option to query planner config

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.
46 changes: 45 additions & 1 deletion internals-js/src/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
SelectionSetNode,
OperationTypeNode,
NameNode,
print,
} from "graphql";
import {
baseType,
Expand Down Expand Up @@ -973,6 +974,21 @@ export class Operation {
return this.withUpdatedSelectionSetAndFragments(optimizedSelection, finalFragments ?? undefined);
}

generateQueryFragments(): Operation {
const fragments = new NamedFragments();
const fragmentSpreadsById = new Map<string, NamedFragmentDefinition>();
const minimizedSelectionSet = this.selectionSet.minimizeSelectionSet(fragments, fragmentSpreadsById);

return new Operation(
this.schema,
this.rootKind,
minimizedSelectionSet,
this.variableDefinitions,
fragments,
this.name,
);
}

expandAllFragments(): Operation {
// We clear up the fragments since we've expanded all.
// Also note that expanding fragment usually generate unecessary fragments/inefficient selections, so it
Expand Down Expand Up @@ -1535,6 +1551,34 @@ export class SelectionSet {
this._selections = mapValues(keyedSelections);
}

minimizeSelectionSet(namedFragments: NamedFragments, fragmentDefinitionsById: Map<string, NamedFragmentDefinition>): SelectionSet {
return this.lazyMap((selection) => {
if (selection.kind === 'FragmentSelection' && selection.element.typeCondition && selection.element.appliedDirectives.length === 0 && selection.selectionSet) {
const id = print(selection.toSelectionNode());
dariuszkuc marked this conversation as resolved.
Show resolved Hide resolved
const existingFragmentDefinition = fragmentDefinitionsById.get(id);
if (existingFragmentDefinition) {
return new FragmentSpreadSelection(this.parentType, namedFragments, existingFragmentDefinition, [])
}

const minimizedSelectionSet = selection.selectionSet.minimizeSelectionSet(namedFragments, fragmentDefinitionsById);
const fragmentDefinition = new NamedFragmentDefinition(
this.parentType.schema(),
`qp__${fragmentDefinitionsById.size}`,
selection.element.typeCondition
).setSelectionSet(minimizedSelectionSet);
const fragmentSpread = new FragmentSpreadSelection(this.parentType, namedFragments, fragmentDefinition, []);
trevor-scheer marked this conversation as resolved.
Show resolved Hide resolved
fragmentDefinitionsById.set(id, fragmentDefinition);
namedFragments.add(fragmentDefinition);
return fragmentSpread;
} else if (selection.kind === 'FieldSelection') {
if (selection.selectionSet) {
selection = selection.withUpdatedSelectionSet(selection.selectionSet.minimizeSelectionSet(namedFragments, fragmentDefinitionsById));
}
}
return selection;
});
}

selectionsInReverseOrder(): readonly Selection[] {
const length = this._selections.length;
const reversed = new Array<Selection>(length);
Expand Down Expand Up @@ -2413,7 +2457,7 @@ export function selectionOfElement(element: OperationElement, subSelection?: Sel
return element.kind === 'Field' ? new FieldSelection(element, subSelection) : new InlineFragmentSelection(element, subSelection!);
}

export type Selection = FieldSelection | FragmentSelection;
export type Selection = FieldSelection | FragmentSelection | FragmentSpreadSelection;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FragmentSelection is an abstract class implemented by InlineFragmentSelection and FragmentSpreadSelection, so I don't think you'll need to change the definition here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

abstract class AbstractSelection<TElement extends OperationElement, TIsLeaf extends undefined | never, TOwnType extends AbstractSelection<TElement, TIsLeaf, TOwnType>> {
constructor(
readonly element: TElement,
Expand Down
156 changes: 156 additions & 0 deletions query-planner-js/src/__tests__/buildPlan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5042,6 +5042,162 @@ describe('Named fragments preservation', () => {
});
});

describe('Fragment autogeneration', () => {
it('respects generateQueryFragments option', () => {
const subgraph1 = {
name: 'Subgraph1',
typeDefs: gql`
type Query {
t: T
}

union T = A | B

type A {
x: Int
y: Int
}

type B {
z: Int
}
`,
};

const [api, queryPlanner] = composeAndCreatePlannerWithOptions(
[subgraph1],
{ generateQueryFragments: true },
);
const operation = operationFromDocument(
api,
gql`
query {
t {
... on A {
x
y
}
... on B {
z
}
}
}
`,
);

const plan = queryPlanner.buildQueryPlan(operation);

expect(plan).toMatchInlineSnapshot(`
QueryPlan {
Fetch(service: "Subgraph1") {
{
t {
__typename
...qp__0
...qp__1
}
}

fragment qp__0 on A {
x
y
}

fragment qp__1 on B {
z
}
},
}
`);
});

it('handles nested fragment generation', () => {
const subgraph1 = {
name: 'Subgraph1',
typeDefs: gql`
type Query {
t: T
}

union T = A | B

type A {
x: Int
y: Int
t: T
}

type B {
z: Int
}
`,
};

const [api, queryPlanner] = composeAndCreatePlannerWithOptions(
[subgraph1],
{ generateQueryFragments: true },
);
const operation = operationFromDocument(
api,
gql`
query {
t {
... on A {
x
y
t {
... on A {
x
}
... on B {
z
}
}
}
... on B {
z
}
}
}
`,
);

const plan = queryPlanner.buildQueryPlan(operation);

expect(plan).toMatchInlineSnapshot(`
QueryPlan {
Fetch(service: "Subgraph1") {
{
t {
__typename
...qp__2
...qp__1
}
}

fragment qp__0 on A {
x
}

fragment qp__1 on B {
z
}

fragment qp__2 on A {
x
y
t {
__typename
...qp__0
...qp__1
}
}
},
}
`);
});
});

test('works with key chains', () => {
const subgraph1 = {
name: 'Subgraph1',
Expand Down
40 changes: 32 additions & 8 deletions query-planner-js/src/buildPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ class QueryPlanningTraversal<RV extends Vertex> {
private newDependencyGraph(): FetchDependencyGraph {
const { supergraphSchema, federatedQueryGraph } = this.parameters;
const rootType = this.isTopLevel && this.hasDefers ? supergraphSchema.schemaDefinition.rootType(this.rootKind) : undefined;
return FetchDependencyGraph.create(supergraphSchema, federatedQueryGraph, this.startFetchIdGen, rootType);
return FetchDependencyGraph.create(supergraphSchema, federatedQueryGraph, this.startFetchIdGen, rootType, this.parameters.config.generateQueryFragments);
}

// Moves the first closed branch to after any branch having more options.
Expand Down Expand Up @@ -907,6 +907,7 @@ class FetchGroup {
// key for that. Having it here saves us from re-computing it more than once.
readonly subgraphAndMergeAtKey?: string,
private cachedCost?: number,
private generateQueryFragments: boolean = false,
// Cache used to save unecessary recomputation of the `isUseless` method.
private isKnownUseful: boolean = false,
private readonly inputRewrites: FetchDataRewrite[] = [],
Expand Down Expand Up @@ -934,6 +935,7 @@ class FetchGroup {
hasInputs,
mergeAt,
deferRef,
generateQueryFragments,
}: {
dependencyGraph: FetchDependencyGraph,
index: number,
Expand All @@ -943,6 +945,7 @@ class FetchGroup {
hasInputs: boolean,
mergeAt?: ResponsePath,
deferRef?: string,
generateQueryFragments: boolean,
}): FetchGroup {
// Sanity checks that the selection parent type belongs to the schema of the subgraph we're querying.
assert(parentType.schema() === dependencyGraph.subgraphSchemas.get(subgraphName), `Expected parent type ${parentType} to belong to ${subgraphName}`);
Expand All @@ -957,7 +960,9 @@ class FetchGroup {
hasInputs ? new GroupInputs(dependencyGraph.supergraphSchema) : undefined,
mergeAt,
deferRef,
hasInputs ? `${toValidGraphQLName(subgraphName)}-${mergeAt?.join('::') ?? ''}` : undefined
hasInputs ? `${toValidGraphQLName(subgraphName)}-${mergeAt?.join('::') ?? ''}` : undefined,
undefined,
generateQueryFragments,
);
}

Expand All @@ -976,6 +981,7 @@ class FetchGroup {
this.deferRef,
this.subgraphAndMergeAtKey,
this.cachedCost,
this.generateQueryFragments,
this.isKnownUseful,
[...this.inputRewrites],
);
Expand Down Expand Up @@ -1516,7 +1522,11 @@ class FetchGroup {
operationName,
);

operation = operation.optimize(fragments?.forSubgraph(this.subgraphName, subgraphSchema));
if (this.generateQueryFragments) {
operation = operation.generateQueryFragments();
} else {
operation = operation.optimize(fragments?.forSubgraph(this.subgraphName, subgraphSchema));
}

const operationDocument = operationToDocument(operation);
const fetchNode: FetchNode = {
Expand Down Expand Up @@ -2060,11 +2070,12 @@ class FetchDependencyGraph {
private readonly rootGroups: MapWithCachedArrays<string, FetchGroup>,
readonly groups: FetchGroup[],
readonly deferTracking: DeferTracking,
readonly generateQueryFragments: boolean,
) {
this.fetchIdGen = startingIdGen;
}

static create(supergraphSchema: Schema, federatedQueryGraph: QueryGraph, startingIdGen: number, rootTypeForDefer: CompositeType | undefined) {
static create(supergraphSchema: Schema, federatedQueryGraph: QueryGraph, startingIdGen: number, rootTypeForDefer: CompositeType | undefined, generateQueryFragments: boolean) {
return new FetchDependencyGraph(
supergraphSchema,
federatedQueryGraph.sources,
Expand All @@ -2073,6 +2084,7 @@ class FetchDependencyGraph {
new MapWithCachedArrays(),
[],
DeferTracking.empty(rootTypeForDefer),
generateQueryFragments,
);
}

Expand All @@ -2097,6 +2109,7 @@ class FetchDependencyGraph {
new MapWithCachedArrays<string, FetchGroup>(),
new Array(this.groups.length),
this.deferTracking.clone(),
this.generateQueryFragments,
);

for (const group of this.groups) {
Expand Down Expand Up @@ -2186,6 +2199,7 @@ class FetchDependencyGraph {
hasInputs,
mergeAt,
deferRef,
generateQueryFragments: this.generateQueryFragments,
});
this.groups.push(newGroup);
return newGroup;
Expand Down Expand Up @@ -3485,9 +3499,9 @@ function computeRootParallelBestPlan(
function createEmptyPlan(
parameters: PlanningParameters<RootVertex>,
): [FetchDependencyGraph, OpPathTree<RootVertex>, number] {
const { supergraphSchema, federatedQueryGraph, root } = parameters;
const { supergraphSchema, federatedQueryGraph, root, config } = parameters;
return [
FetchDependencyGraph.create(supergraphSchema, federatedQueryGraph, 0, undefined),
FetchDependencyGraph.create(supergraphSchema, federatedQueryGraph, 0, undefined, config.generateQueryFragments),
PathTree.createOp(federatedQueryGraph, root),
0
];
Expand Down Expand Up @@ -3524,7 +3538,17 @@ function computeRootSerialDependencyGraph(
// }
// then we should _not_ merge the 2 `mut1` fields (contrarily to what happens on queried fields).
prevPaths = prevPaths.concat(newPaths);
prevDepGraph = computeRootFetchGroups(FetchDependencyGraph.create(supergraphSchema, federatedQueryGraph, startingFetchId, rootType), prevPaths, root.rootKind);
prevDepGraph = computeRootFetchGroups(
FetchDependencyGraph.create(
supergraphSchema,
federatedQueryGraph,
startingFetchId,
rootType,
parameters.config.generateQueryFragments,
),
prevPaths,
root.rootKind,
);
} else {
startingFetchId = prevDepGraph.nextFetchId();
graphs.push(prevDepGraph);
Expand Down Expand Up @@ -3805,7 +3829,7 @@ function computeNonRootFetchGroups(dependencyGraph: FetchDependencyGraph, pathTr
// The edge tail type is one of the subgraph root type, so it has to be an ObjectType.
const rootType = pathTree.vertex.type;
assert(isCompositeType(rootType), () => `Should not have condition on non-selectable type ${rootType}`);
const group = dependencyGraph.getOrCreateRootFetchGroup({ subgraphName, rootKind, parentType: rootType} );
const group = dependencyGraph.getOrCreateRootFetchGroup({ subgraphName, rootKind, parentType: rootType } );
computeGroupsForTree(dependencyGraph, pathTree, group, GroupPath.empty(), emptyDeferContext);
return dependencyGraph;
}
Expand Down
Loading