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

Support optimized select for top-level query #2235

Draft
wants to merge 46 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
2f13537
optimize select
Lyrisbee Nov 11, 2022
290e464
add reference
Lyrisbee Nov 11, 2022
03d4124
optimize find directive and fix tests
Lyrisbee Nov 11, 2022
cacc576
add select directvie
Lyrisbee Nov 13, 2022
d49f0ec
move select directive
Lyrisbee Nov 13, 2022
6f1a04a
add optimize select config
Lyrisbee Nov 13, 2022
ed0bee9
filter non-array document
Lyrisbee Nov 15, 2022
3ef48bd
remove Schema request
Lyrisbee Nov 15, 2022
23237a1
handle RelationOrderBy problems
Lyrisbee Nov 16, 2022
1b73b78
lint
Lyrisbee Nov 16, 2022
bcf8f8c
add tests
Lyrisbee Nov 21, 2022
6ca50ae
rename tests
Lyrisbee Nov 21, 2022
c9a7343
use str_replace
Lyrisbee Nov 21, 2022
7e75f24
Apply php-cs-fixer changes
Lyrisbee Nov 21, 2022
45d12f1
lint
bepsvpt Nov 21, 2022
cd30eea
Apply php-cs-fixer changes
bepsvpt Nov 21, 2022
fa6f99a
add old version compatibility
Lyrisbee Nov 21, 2022
7bc7c76
fix localKeyName for laravel version below 5.7
bepsvpt Nov 21, 2022
f983dec
Apply php-cs-fixer changes
bepsvpt Nov 21, 2022
25e0ecc
fix reflection issue
bepsvpt Nov 21, 2022
abf875f
Apply php-cs-fixer changes
bepsvpt Nov 21, 2022
61ed5cd
fix ReflectionClass getValue
bepsvpt Nov 21, 2022
768b28b
wip, fix tests
bepsvpt Nov 21, 2022
6e21e28
wip, fix tests
bepsvpt Nov 21, 2022
0990919
cleanup debug code
bepsvpt Nov 21, 2022
deabb67
lint
Lyrisbee Nov 22, 2022
667f222
update select directive doc
Lyrisbee Nov 22, 2022
6b456f6
only `EloquentBuilder` can be optimized and lint
Lyrisbee Nov 22, 2022
c3ff536
use container
Lyrisbee Nov 22, 2022
9ec7c71
update tests
Lyrisbee Nov 22, 2022
ee2442f
remove dead code
Lyrisbee Nov 22, 2022
3221dd2
lint
Lyrisbee Dec 2, 2022
982f4c1
Apply suggestions
Lyrisbee Dec 14, 2022
5cd2304
select without relation directives and fix morphTo
Lyrisbee Dec 15, 2022
e5b0715
cleanup
Lyrisbee Dec 15, 2022
2d9f386
fix paginate schema correlates
Lyrisbee Dec 16, 2022
aafd02c
order by primary key by default
Lyrisbee Dec 16, 2022
f0c4846
throw errors
Lyrisbee Dec 16, 2022
5171228
set default config to false
Lyrisbee Dec 16, 2022
1d570bf
fix isRelation for laravel version below 8.0
Lyrisbee Dec 16, 2022
f385f5d
fix relations
Lyrisbee Dec 16, 2022
b170698
Merge branch 'master' into optimize-query
Lyrisbee Dec 16, 2022
2aa0e4a
fix tests
Lyrisbee Dec 16, 2022
f4fb5d8
Apply php-cs-fixer changes
Lyrisbee Dec 16, 2022
24ee8d4
cleanup
Lyrisbee Dec 16, 2022
0635733
set primary key if no select column
Lyrisbee Dec 19, 2022
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
42 changes: 42 additions & 0 deletions src/Pagination/PaginateDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,20 @@

namespace Nuwave\Lighthouse\Pagination;

use GraphQL\Error\Error;
use GraphQL\Language\AST\FieldDefinitionNode;
use GraphQL\Language\AST\ObjectTypeDefinitionNode;
use Illuminate\Contracts\Pagination\Paginator;
use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
use Illuminate\Database\Eloquent\Relations\Relation;
use Illuminate\Database\Query\Builder as QueryBuilder;
use Illuminate\Support\Arr;
use Laravel\Scout\Builder as ScoutBuilder;
use Nuwave\Lighthouse\Execution\ResolveInfo;
use Nuwave\Lighthouse\Schema\AST\DocumentAST;
use Nuwave\Lighthouse\Schema\Directives\BaseDirective;
use Nuwave\Lighthouse\Schema\Values\FieldValue;
use Nuwave\Lighthouse\Select\SelectHelper;
use Nuwave\Lighthouse\Support\Contracts\FieldManipulator;
use Nuwave\Lighthouse\Support\Contracts\FieldResolver;
use Nuwave\Lighthouse\Support\Contracts\GraphQLContext;
Expand Down Expand Up @@ -156,6 +159,45 @@ public function resolveField(FieldValue $fieldValue): FieldValue
$this->directiveArgValue('scopes', [])
);

if (config('lighthouse.optimized_selects')) {
if ($query instanceof EloquentBuilder) {
$fieldSelection = $resolveInfo->getFieldSelection(2);

if (($hasData = Arr::has($fieldSelection, 'data')) || Arr::has($fieldSelection, 'edges')) {
$data = $hasData
? $fieldSelection['data']
: $fieldSelection['edges']['node'];
Comment on lines +166 to +169
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can derive which field to check from $this->directiveArgValue('type'), no need to check both.


/** @var array<int, string> $fieldSelection */
$fieldSelection = array_keys($data);

$model = $query->getModel();

$selectColumns = SelectHelper::getSelectColumns(
$this->definitionNode,
$fieldSelection,
get_class($model)
);

if (empty($selectColumns)) {
throw new Error('The select column is empty.');
}

$query = $query->select($selectColumns);

/** @var string|string[] $keyName */
$keyName = $model->getKeyName();
if (is_string($keyName)) {
$keyName = [$keyName];
}

foreach ($keyName as $name) {
$query->orderBy($name);
}
Comment on lines +188 to +196
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be unrelated to optimizing the select.

}
}
}

$paginationArgs = PaginationArgs::extractArgs($args, $this->paginationType(), $this->paginateMaxCount());

$paginationArgs->type = $this->optimalPaginationType($resolveInfo);
Expand Down
13 changes: 13 additions & 0 deletions src/Pagination/PaginationManipulator.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use GraphQL\Language\AST\FieldDefinitionNode;
use GraphQL\Language\AST\NamedTypeNode;
use GraphQL\Language\AST\Node;
use GraphQL\Language\AST\NonNullTypeNode;
use GraphQL\Language\AST\ObjectTypeDefinitionNode;
use GraphQL\Language\AST\TypeNode;
Expand All @@ -15,6 +16,8 @@
use Nuwave\Lighthouse\Schema\AST\DocumentAST;
use Nuwave\Lighthouse\Schema\Directives\ModelDirective;

use function Safe\preg_replace;

class PaginationManipulator
{
/**
Expand Down Expand Up @@ -273,4 +276,14 @@ protected function paginationResultType(string $typeName): TypeNode

return $typeNode;
}

public static function getReturnTypeName(Node $fieldDefinition): ?string
{
if (! ASTHelper::directiveDefinition($fieldDefinition, 'paginate')) {
return null;
}
$fieldTypeName = ASTHelper::getUnderlyingTypeName($fieldDefinition);

return preg_replace('/(Connection|SimplePaginator|Paginator)$/', '', $fieldTypeName);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can look at the argument of @paginate(type: ?) to know exactly which one to remove. That way, a model called ProductSimple of type PAGINATOR will not wrongly be seen as Product.

}
}
54 changes: 51 additions & 3 deletions src/Schema/Directives/AllDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@

namespace Nuwave\Lighthouse\Schema\Directives;

use GraphQL\Error\Error;
use GraphQL\Language\AST\FieldDefinitionNode;
use GraphQL\Language\AST\ObjectTypeDefinitionNode;
use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
use Illuminate\Database\Eloquent\Relations\Relation;
use Illuminate\Database\Query\Builder as QueryBuilder;
use Illuminate\Database\Query\Expression;
use Illuminate\Support\Collection;
use Laravel\Scout\Builder as ScoutBuilder;
use Nuwave\Lighthouse\Execution\ResolveInfo;
use Nuwave\Lighthouse\Schema\AST\DocumentAST;
use Nuwave\Lighthouse\Schema\Values\FieldValue;
use Nuwave\Lighthouse\Select\SelectHelper;
use Nuwave\Lighthouse\Support\Contracts\FieldManipulator;
use Nuwave\Lighthouse\Support\Contracts\FieldResolver;
use Nuwave\Lighthouse\Support\Contracts\GraphQLContext;
Expand Down Expand Up @@ -63,12 +66,57 @@ public function resolveField(FieldValue $fieldValue): FieldValue
$query = $this->getModelClass()::query();
}

return $resolveInfo
$builder = $resolveInfo
->enhanceBuilder(
$query,
$this->directiveArgValue('scopes', [])
)
->get();
);

if (config('lighthouse.optimized_selects')) {
if ($builder instanceof EloquentBuilder) {
$fieldSelection = array_keys($resolveInfo->getFieldSelection(1));

$model = $builder->getModel();

$selectColumns = SelectHelper::getSelectColumns(
$this->definitionNode,
$fieldSelection,
get_class($model)
);

if (empty($selectColumns)) {
throw new Error('The select column is empty.');
}

$query = $builder->getQuery();

if (null !== $query->columns) {
$bindings = $query->getRawBindings();

$expressions = array_filter($query->columns, function ($column) {
return $column instanceof Expression;
});

$builder = $builder->select(array_unique(array_merge($selectColumns, $expressions)));

$builder = $builder->addBinding($bindings['select'], 'select');
} else {
$builder = $builder->select($selectColumns);
}

/** @var string|string[] $keyName */
$keyName = $model->getKeyName();
if (is_string($keyName)) {
$keyName = [$keyName];
}

foreach ($keyName as $name) {
$query->orderBy($name);
}
Comment on lines +107 to +115
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, this seems like an unnecessary addition that is orthogonal to optimizing select.

}
}

return $builder->get();
});

return $fieldValue;
Expand Down
36 changes: 33 additions & 3 deletions src/Schema/Directives/FindDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Illuminate\Database\Eloquent\Model;
use Nuwave\Lighthouse\Execution\ResolveInfo;
use Nuwave\Lighthouse\Schema\Values\FieldValue;
use Nuwave\Lighthouse\Select\SelectHelper;
use Nuwave\Lighthouse\Support\Contracts\FieldResolver;
use Nuwave\Lighthouse\Support\Contracts\GraphQLContext;

Expand Down Expand Up @@ -35,12 +36,41 @@ public static function definition(): string
public function resolveField(FieldValue $fieldValue): FieldValue
{
$fieldValue->setResolver(function ($root, array $args, GraphQLContext $context, ResolveInfo $resolveInfo): ?Model {
$results = $resolveInfo
$builder = $resolveInfo
->enhanceBuilder(
$this->getModelClass()::query(),
$this->directiveArgValue('scopes', [])
)
->get();
);

if (config('lighthouse.optimized_selects')) {
$fieldSelection = array_keys($resolveInfo->getFieldSelection(1));

$selectColumns = SelectHelper::getSelectColumns(
$this->definitionNode,
$fieldSelection,
$this->getModelClass()
);

if (empty($selectColumns)) {
throw new Error('The select column is empty.');
}

$builder = $builder->select($selectColumns);

$model = $builder->getModel();

/** @var string|string[] $keyName */
$keyName = $model->getKeyName();
if (is_string($keyName)) {
$keyName = [$keyName];
}

foreach ($keyName as $name) {
$builder->orderBy($name);
}
Comment on lines +61 to +70
Copy link
Collaborator

Choose a reason for hiding this comment

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

unnecessary

}

$results = $builder->get();

if ($results->count() > 1) {
throw new Error('The query returned more than one result.');
Expand Down
21 changes: 21 additions & 0 deletions src/Schema/Directives/SelectDirective.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

namespace Nuwave\Lighthouse\Schema\Directives;

class SelectDirective extends BaseDirective
{
public static function definition(): string
{
return /** @lang GraphQL */ <<<'GRAPHQL'
"""
Specify the SQL column dependencies of this field.
"""
directive @select(
"""
SQL column names to include in the `SELECT` part of the query.
"""
columns: [String!]!
) on FIELD_DEFINITION
GRAPHQL;
}
}
Loading