Skip to content

Commit bfa78b4

Browse files
spawniaspawnia
authored
Improve QueryPlan (#1297)
1 parent ff5bc60 commit bfa78b4

File tree

3 files changed

+136
-77
lines changed

3 files changed

+136
-77
lines changed

‎CHANGELOG.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,16 @@ You can find and compare releases at the [GitHub release page](https://github.co
99

1010
## Unreleased
1111

12+
## v15.0.3
13+
14+
### Fixed
15+
16+
- Fix `QueryPlan` for `union` types
17+
18+
### Changed
19+
20+
- Improve `QueryPlan` performance
21+
1222
## v15.0.2
1323

1424
### Fixed

‎src/Type/Definition/QueryPlan.php‎

Lines changed: 52 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,17 @@
1414

1515
/**
1616
* @phpstan-type QueryPlanOptions array{
17-
* groupImplementorFields?: bool
17+
* groupImplementorFields?: bool,
1818
* }
1919
*/
2020
class QueryPlan
2121
{
22-
/** @var array<string, array<int, string>> */
23-
private array $types = [];
22+
/**
23+
* Map from type names to a list of fields referenced of that type.
24+
*
25+
* @var array<string, array<string, true>>
26+
*/
27+
private array $typeToFields = [];
2428

2529
private Schema $schema;
2630

@@ -63,38 +67,50 @@ public function queryPlan(): array
6367
*/
6468
public function getReferencedTypes(): array
6569
{
66-
return \array_keys($this->types);
70+
return \array_keys($this->typeToFields);
6771
}
6872

6973
public function hasType(string $type): bool
7074
{
71-
return isset($this->types[$type]);
75+
return isset($this->typeToFields[$type]);
7276
}
7377

7478
/**
79+
* TODO return array<string, true>.
80+
*
7581
* @return array<int, string>
7682
*/
7783
public function getReferencedFields(): array
7884
{
79-
return \array_values(\array_unique(\array_merge(...\array_values($this->types))));
85+
$allFields = [];
86+
foreach ($this->typeToFields as $fields) {
87+
foreach ($fields as $field => $_) {
88+
$allFields[$field] = true;
89+
}
90+
}
91+
92+
return array_keys($allFields);
8093
}
8194

8295
public function hasField(string $field): bool
8396
{
84-
return \count(
85-
\array_filter(
86-
$this->getReferencedFields(),
87-
static fn (string $referencedField): bool => $field === $referencedField
88-
)
89-
) > 0;
97+
foreach ($this->typeToFields as $fields) {
98+
if (array_key_exists($field, $fields)) {
99+
return true;
100+
}
101+
}
102+
103+
return false;
90104
}
91105

92106
/**
107+
* TODO return array<string, true>.
108+
*
93109
* @return array<int, string>
94110
*/
95111
public function subFields(string $typename): array
96112
{
97-
return $this->types[$typename] ?? [];
113+
return array_keys($this->typeToFields[$typename] ?? []);
98114
}
99115

100116
/**
@@ -112,19 +128,10 @@ private function analyzeQueryPlan(ObjectType $parentType, iterable $fieldNodes):
112128
$type = Type::getNamedType(
113129
$parentType->getField($fieldNode->name->value)->getType()
114130
);
115-
assert($type instanceof ObjectType || $type instanceof InterfaceType, 'proven because it must be a type with fields and was unwrapped');
131+
assert($type instanceof Type, 'known because schema validation');
116132

117133
$subfields = $this->analyzeSelectionSet($fieldNode->selectionSet, $type, $implementors);
118-
119-
$this->types[$type->name] = \array_unique(\array_merge(
120-
\array_key_exists($type->name, $this->types) ? $this->types[$type->name] : [],
121-
\array_keys($subfields)
122-
));
123-
124-
$queryPlan = $this->arrayMergeDeep(
125-
$queryPlan,
126-
$subfields
127-
);
134+
$queryPlan = $this->arrayMergeDeep($queryPlan, $subfields);
128135
}
129136

130137
if ($this->groupImplementorFields) {
@@ -199,6 +206,15 @@ private function analyzeSelectionSet(SelectionSetNode $selectionSet, Type $paren
199206
}
200207
}
201208

209+
$parentTypeName = $parentType->name();
210+
211+
// TODO evaluate if this line is really necessary - it causes abstract types to appear
212+
// in getReferencedTypes() even if they do not have any fields directly referencing them.
213+
$this->typeToFields[$parentTypeName] ??= [];
214+
foreach ($fields as $fieldName => $_) {
215+
$this->typeToFields[$parentTypeName][$fieldName] = true;
216+
}
217+
202218
return $fields;
203219
}
204220

@@ -211,16 +227,9 @@ private function analyzeSubFields(Type $type, SelectionSetNode $selectionSet, ar
211227
{
212228
$type = Type::getNamedType($type);
213229

214-
$subfields = [];
215-
if ($type instanceof ObjectType || $type instanceof AbstractType) {
216-
$subfields = $this->analyzeSelectionSet($selectionSet, $type, $implementors);
217-
$this->types[$type->name] = \array_unique(\array_merge(
218-
\array_key_exists($type->name, $this->types) ? $this->types[$type->name] : [],
219-
\array_keys($subfields)
220-
));
221-
}
222-
223-
return $subfields;
230+
return $type instanceof ObjectType || $type instanceof AbstractType
231+
? $this->analyzeSelectionSet($selectionSet, $type, $implementors)
232+
: [];
224233
}
225234

226235
/**
@@ -248,18 +257,15 @@ private function mergeFields(Type $parentType, Type $type, array $fields, array
248257
\array_intersect_key($subfields, $fields)
249258
);
250259
} else {
251-
$fields = $this->arrayMergeDeep(
252-
$subfields,
253-
$fields
254-
);
260+
$fields = $this->arrayMergeDeep($subfields, $fields);
255261
}
256262

257263
return $fields;
258264
}
259265

260266
/**
261-
* similar to array_merge_recursive this merges nested arrays, but handles non array values differently
262-
* while array_merge_recursive tries to merge non array values, in this implementation they will be overwritten.
267+
* Merges nested arrays, but handles non array values differently from array_merge_recursive.
268+
* While array_merge_recursive tries to merge non-array values, in this implementation they will be overwritten.
263269
*
264270
* @see https://stackoverflow.com/a/25712428
265271
*
@@ -270,20 +276,18 @@ private function mergeFields(Type $parentType, Type $type, array $fields, array
270276
*/
271277
private function arrayMergeDeep(array $array1, array $array2): array
272278
{
273-
$merged = $array1;
274-
275279
foreach ($array2 as $key => &$value) {
276280
if (\is_numeric($key)) {
277-
if (! \in_array($value, $merged, true)) {
278-
$merged[] = $value;
281+
if (! \in_array($value, $array1, true)) {
282+
$array1[] = $value;
279283
}
280-
} elseif (\is_array($value) && isset($merged[$key]) && \is_array($merged[$key])) {
281-
$merged[$key] = $this->arrayMergeDeep($merged[$key], $value);
284+
} elseif (\is_array($value) && isset($array1[$key]) && \is_array($array1[$key])) {
285+
$array1[$key] = $this->arrayMergeDeep($array1[$key], $value);
282286
} else {
283-
$merged[$key] = $value;
287+
$array1[$key] = $value;
284288
}
285289
}
286290

287-
return $merged;
291+
return $array1;
288292
}
289293
}

‎tests/Type/QueryPlanTest.php‎

Lines changed: 74 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -273,12 +273,12 @@ public function testQueryPlan(): void
273273
$schema = new Schema(['query' => $blogQuery]);
274274
$result = GraphQL::executeQuery($schema, $doc)->toArray();
275275

276-
self::assertEquals(['data' => ['article' => null]], $result);
276+
self::assertSame(['data' => ['article' => null]], $result);
277277
self::assertInstanceOf(QueryPlan::class, $queryPlan);
278278
self::assertEquals($expectedQueryPlan, $queryPlan->queryPlan());
279-
self::assertEquals($expectedReferencedTypes, $queryPlan->getReferencedTypes());
280-
self::assertEquals($expectedReferencedFields, $queryPlan->getReferencedFields());
281-
self::assertEquals(['url', 'width', 'height'], $queryPlan->subFields('Image'));
279+
self::assertSame($expectedReferencedTypes, $queryPlan->getReferencedTypes());
280+
self::assertSame($expectedReferencedFields, $queryPlan->getReferencedFields());
281+
self::assertSame(['url', 'width', 'height'], $queryPlan->subFields('Image'));
282282

283283
self::assertTrue($queryPlan->hasField('url'));
284284
self::assertFalse($queryPlan->hasField('test'));
@@ -388,25 +388,15 @@ public function testQueryPlanOnInterface(): void
388388
'name',
389389
];
390390

391-
/** @var QueryPlan $queryPlan */
391+
/** @var QueryPlan|null $queryPlan */
392392
$queryPlan = null;
393-
$hasCalled = false;
394393

395394
$petsQuery = new ObjectType([
396395
'name' => 'Query',
397396
'fields' => [
398397
'pets' => [
399398
'type' => Type::listOf($petType),
400-
'resolve' => static function (
401-
$value,
402-
$args,
403-
$context,
404-
ResolveInfo $info
405-
) use (
406-
&$hasCalled,
407-
&$queryPlan
408-
): array {
409-
$hasCalled = true;
399+
'resolve' => static function ($value, array $args, $context, ResolveInfo $info) use (&$queryPlan): array {
410400
$queryPlan = $info->lookAhead();
411401

412402
return [];
@@ -428,11 +418,11 @@ public function testQueryPlanOnInterface(): void
428418
]);
429419
GraphQL::executeQuery($schema, $query)->toArray();
430420

431-
self::assertTrue($hasCalled);
421+
self::assertInstanceOf(QueryPlan::class, $queryPlan);
432422
self::assertEquals($expectedQueryPlan, $queryPlan->queryPlan());
433-
self::assertEquals($expectedReferencedTypes, $queryPlan->getReferencedTypes());
434-
self::assertEquals($expectedReferencedFields, $queryPlan->getReferencedFields());
435-
self::assertEquals(['woofs'], $queryPlan->subFields('Dog'));
423+
self::assertSame($expectedReferencedTypes, $queryPlan->getReferencedTypes());
424+
self::assertSame($expectedReferencedFields, $queryPlan->getReferencedFields());
425+
self::assertSame(['woofs'], $queryPlan->subFields('Dog'));
436426

437427
self::assertTrue($queryPlan->hasField('name'));
438428
self::assertFalse($queryPlan->hasField('test'));
@@ -441,6 +431,61 @@ public function testQueryPlanOnInterface(): void
441431
self::assertFalse($queryPlan->hasType('Test'));
442432
}
443433

434+
public function testQueryPlanTypenameOnUnion(): void
435+
{
436+
$dogType = new ObjectType([
437+
'name' => 'Dog',
438+
'isTypeOf' => static fn ($obj): bool => $obj instanceof Dog,
439+
'fields' => static fn (): array => [
440+
'name' => ['type' => Type::string()],
441+
],
442+
]);
443+
444+
$petType = new UnionType([
445+
'name' => 'Pet',
446+
'types' => [$dogType],
447+
]);
448+
449+
/** @var QueryPlan|null $queryPlan */
450+
$queryPlan = null;
451+
$petsQuery = new ObjectType([
452+
'name' => 'Query',
453+
'fields' => [
454+
'pets' => [
455+
'type' => Type::listOf($petType),
456+
'resolve' => static function ($value, array $args, $context, ResolveInfo $info) use (&$queryPlan): array {
457+
$queryPlan = $info->lookAhead();
458+
459+
return [];
460+
},
461+
],
462+
],
463+
]);
464+
465+
$schema = new Schema(['query' => $petsQuery]);
466+
GraphQL::executeQuery($schema, /** @lang GraphQL */ '
467+
{
468+
pets {
469+
__typename
470+
}
471+
}
472+
');
473+
474+
self::assertInstanceOf(QueryPlan::class, $queryPlan);
475+
self::assertSame([], $queryPlan->queryPlan());
476+
self::assertSame(['Pet'], $queryPlan->getReferencedTypes());
477+
self::assertSame([], $queryPlan->getReferencedFields());
478+
self::assertSame([], $queryPlan->subFields('Dog'));
479+
480+
// TODO really? maybe change in next major version
481+
self::assertFalse($queryPlan->hasField('__typename'));
482+
self::assertFalse($queryPlan->hasField('non-existent'));
483+
484+
self::assertTrue($queryPlan->hasType('Pet'));
485+
self::assertFalse($queryPlan->hasType('Dog'));
486+
self::assertFalse($queryPlan->hasType('Non-Existent'));
487+
}
488+
444489
public function testMergedFragmentsQueryPlan(): void
445490
{
446491
$image = new ObjectType([
@@ -719,11 +764,11 @@ public function testMergedFragmentsQueryPlan(): void
719764
$result = GraphQL::executeQuery($schema, $doc)->toArray();
720765

721766
self::assertTrue($hasCalled);
722-
self::assertEquals(['data' => ['article' => null]], $result);
767+
self::assertSame(['data' => ['article' => null]], $result);
723768
self::assertEquals($expectedQueryPlan, $queryPlan->queryPlan());
724-
self::assertEquals($expectedReferencedTypes, $queryPlan->getReferencedTypes());
725-
self::assertEquals($expectedReferencedFields, $queryPlan->getReferencedFields());
726-
self::assertEquals(['url', 'width', 'height'], $queryPlan->subFields('Image'));
769+
self::assertSame($expectedReferencedTypes, $queryPlan->getReferencedTypes());
770+
self::assertSame($expectedReferencedFields, $queryPlan->getReferencedFields());
771+
self::assertSame(['url', 'width', 'height'], $queryPlan->subFields('Image'));
727772

728773
self::assertTrue($queryPlan->hasField('url'));
729774
self::assertFalse($queryPlan->hasField('test'));
@@ -943,12 +988,12 @@ public function testQueryPlanGroupingImplementorFieldsForAbstractTypes(): void
943988
$result = GraphQL::executeQuery($schema, $query)->toArray();
944989

945990
self::assertTrue($hasCalled);
946-
self::assertEquals($expectedResult, $result);
947-
self::assertEquals($expectedQueryPlan, $queryPlan->queryPlan());
991+
self::assertSame($expectedResult, $result);
992+
self::assertSame($expectedQueryPlan, $queryPlan->queryPlan());
948993
self::assertEquals($expectedReferencedTypes, $queryPlan->getReferencedTypes());
949-
self::assertEquals($expectedReferencedFields, $queryPlan->getReferencedFields());
950-
self::assertEquals($expectedItemSubFields, $queryPlan->subFields('Item'));
951-
self::assertEquals($expectedBuildingSubFields, $queryPlan->subFields('Building'));
994+
self::assertSame($expectedReferencedFields, $queryPlan->getReferencedFields());
995+
self::assertSame($expectedItemSubFields, $queryPlan->subFields('Item'));
996+
self::assertSame($expectedBuildingSubFields, $queryPlan->subFields('Building'));
952997
}
953998

954999
public function testQueryPlanForMultipleFieldNodes(): void

0 commit comments

Comments
 (0)