rebing / graphql-laravel

Laravel wrapper for Facebook's GraphQL
MIT License
2.13k stars 266 forks source link

Select * is preventing from groupping by for Interface types #683

Open EdwinDayot opened 4 years ago

EdwinDayot commented 4 years ago

Versions:

Description:

The generated SQL for a group by on an interface would raise an error as * is selected instead of the group by fields

Steps To Reproduce:

<?php

class MyQuery extends Query {

    public function type()
    {
        return Type::listOf(GraphQL::type('anInterface'));
    }

    public function resolve($root, $args, $context, $resolveInfo, $getSelectFields)
    {
        $fields = $getSelectFields();
        $select = $fields->getSelect();
        $with = $fields->getRelations();

        $myResults = MyModel::select($select)->with($with)->groupBy('my_field')->get();
    }
}
<?php

class MyInterface extends InterfaceType {
    public function fields(): array
    {
        return [
            'my_field' => ['type' => Type::string()]
        ]
    }
}
<?php

class MyType extends Type {
    public function fields(): array
    {
        return $interface->getFields();
    }
}

With this kind of configuration, the SQL request would result in:

select * from my_table group by my_field;

And of course, it would raise an SQL error: Syntax error or access violation: 1055 Expression #1 of SELECT list is not in GROUP BY clause and contains nonaggregated column ...

Expectations

Only the required fields should be selected

illambo commented 4 years ago

Maybe the problem is this https://github.com/rebing/graphql-laravel/blob/19ea3ed120b96c8c9b45398d0fb1ab1514860a4d/src/Support/SelectFields.php#L256

EdwinDayot commented 4 years ago

I think it's just a part of a larger problem. Each Interface or Union should assume that every field defined in it are available in the database, but should use the alias and query defined in the type implementing the Interface or Union.

I have been working on it with the method

https://github.com/rebing/graphql-laravel/blob/19ea3ed120b96c8c9b45398d0fb1ab1514860a4d/src/Support/SelectFields.php#L449

But it does not support aliases. (apparently)

illambo commented 4 years ago

Great! May the force be with you :)

mfn commented 4 years ago

Added the SelectFields label, seems like another shortcoming with this part.

crissi commented 3 years ago

I am taking a look at this, it seems that relationships are not working well either...

crissi commented 3 years ago

Tried to tackle this by using webonyx QueryPlan helper, which could ease this problem quite a bit, but since it still have unresolved issues, like this: https://github.com/webonyx/graphql-php/pull/831, It will have to wait for now

mfn commented 3 years ago

Does https://github.com/nuwave/lighthouse support? Not sure, might be worth checking out how they do it.