lighthouse
lighthouse copied to clipboard
Deduplicate nested relations using dot notation with equivalent fields
What problem does this feature proposal attempt to solve?
See https://github.com/nuwave/lighthouse/pull/1871. Database queries are unnecessarily duplicated.
Which possible solutions should be considered?
When using dot notation to batch load a relation, e.g. @with(relation: "foo.bar.baz"), we would have to pick apart the individual relation segments and resolve a batch loader for each one separately.
Rough draft of how that might look:
/** @var array<int, \Nuwave\Lighthouse\Execution\BatchLoader\RelationBatchLoader> $relationBatchLoader */
$loaders = [];
// Dot notation may be used to eager load nested relations
$parts = explode('.', $this->relation());
// Includes the field we are loading the relation for
$path = $resolveInfo->path;
// In case we have no args, we can combine eager loads that are the same
if ($args === []) {
array_pop($path);
}
$lastRelation = array_pop($parts);
foreach ($parts as $intermediaryRelation) {
$path []= $intermediaryRelation;
$loaders []= BatchLoaderRegistry::instance(
$intermediaryRelation,
function () use ($resolveInfo, $intermediaryRelation): RelationBatchLoader {
return new RelationBatchLoader($this->relationLoader($resolveInfo, $intermediaryRelation));
}
);
}
$path []= $lastRelation;
$path = array_merge($path, $this->scopes());
$loaders []= BatchLoaderRegistry::instance(
$path,
function () use ($resolveInfo, $lastRelation): RelationBatchLoader {
return new RelationBatchLoader($this->relationLoader($resolveInfo, $lastRelation));
}
);
return new Deferred(static function () use ($loaders, $parent) {
/** @var RelationBatchLoader $loader */
foreach ($loaders as $loader) {
$loader->load($parent);
}
});
Scopes and additional constraints only apply to the last relation query, so the intermediary queries can safely be batched together with other unconstrained queries on the same relation:

Eloquent provides a method to load relations only when they are not loaded already loadMissing. Could it be utilised here?
Unfortunately, changing https://github.com/nuwave/lighthouse/blob/4dd08990889ecf996a2a955434852c967cb9c2ee/src/Execution/ModelsLoader/SimpleModelsLoader.php#L29 to use loadMissing causes many tests to fail - the underlying issue is that relations can be loaded multiple times in a single GraphQL query, but they may apply different conditions.
See failing test logs
1) Tests\Integration\Execution\DataLoader\RelationBatchLoaderTest::testSplitsEagerLoadsByScopes
Failed asserting that 2 matches expected 3.
/workdir/tests/AssertsQueryCounts.php:47
/workdir/tests/Integration/Execution/DataLoader/RelationBatchLoaderTest.php:308
2) Tests\Integration\Execution\DataLoader\RelationBatchLoaderTest::testSplitsEagerLoadsWithArguments
Failed asserting that 2 matches expected 3.
/workdir/tests/AssertsQueryCounts.php:47
/workdir/tests/Integration/Execution/DataLoader/RelationBatchLoaderTest.php:341
3) Tests\Integration\Schema\Directives\BelongsToDirectiveTest::testDoesNotShortcutForeignKeyIfQueryHasConditions
Unable to find JSON:
[{
"data": {
"user": {
"company": null
}
}
}]
within response JSON:
[{
"data": {
"user": {
"company": {
"id": "1"
}
}
}
}].
Failed asserting that an array has the subset Array &0 (
'data' => Array &1 (
'user' => Array &2 (
'company' => null
)
)
).
--- Expected
+++ Actual
@@ @@
array (
'user' =>
array (
- 'company' => NULL,
+ 'company' =>
+ array (
+ 'id' => '1',
+ ),
),
),
)
/workdir/vendor/laravel/framework/src/Illuminate/Testing/Constraints/ArraySubset.php:85
/workdir/vendor/laravel/framework/src/Illuminate/Testing/Assert.php:40
/workdir/vendor/laravel/framework/src/Illuminate/Testing/AssertableJsonString.php:295
/workdir/vendor/laravel/framework/src/Illuminate/Testing/TestResponse.php:673
/workdir/tests/Integration/Schema/Directives/BelongsToDirectiveTest.php:403
/workdir/tests/AssertsQueryCounts.php:44
/workdir/tests/Integration/Schema/Directives/BelongsToDirectiveTest.php:407
4) Tests\Integration\Schema\Directives\HasManyDirectiveTest::testQueryHasManyWithConditionInDifferentAliases
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
"id": 1\n
}\n
],\n
- "lastTasks": []\n
- },\n
- {\n
- "firstTasks": [],\n
"lastTasks": [\n
{\n
- "id": 6\n
+ "id": 1\n
}\n
]\n
+ },\n
+ {\n
+ "firstTasks": [],\n
+ "lastTasks": []\n
}\n
]\n
}\n
}'
/workdir/vendor/laravel/framework/src/Illuminate/Testing/AssertableJsonString.php:102
/workdir/vendor/laravel/framework/src/Illuminate/Testing/TestResponse.php:709
/workdir/tests/Integration/Schema/Directives/HasManyDirectiveTest.php:202
5) Tests\Integration\SoftDeletes\SoftDeletesAndTrashedDirectiveTest::testNested
Failed to assert that the response count matched the expected 2
Failed asserting that actual size 3 matches expected size 2.
/workdir/vendor/laravel/framework/src/Illuminate/Testing/AssertableJsonString.php:74
/workdir/vendor/laravel/framework/src/Illuminate/Testing/TestResponse.php:803
/workdir/tests/Integration/SoftDeletes/SoftDeletesAndTrashedDirectiveTest.php:359