nuwave / lighthouse

A framework for serving GraphQL from Laravel
https://lighthouse-php.com
MIT License
3.37k stars 438 forks source link

Allow SQL expressions in `@orderBy` #2056

Open guillaumeboussion opened 2 years ago

guillaumeboussion commented 2 years ago

What problem does this feature proposal attempt to solve?

This feature would allow requests to be sorted by null column values.

In example : to order a list of DateTime in SQL, providing ISNULL(created_at) ASC, created_at ASC allows date to be ordered right after non-null date values.

For the moment, it's only possible to do it by setting a @builder directive, and chain a raw SQL ordering. So it's ok when ordering on a simple table, but this is not as easy with a relation ordering

Which possible solutions should be considered?

In the OrderByDirection enum, maybe add 2 attributes ? NULL_ASC and NULL_DESC ? I'll stick to what is the simplest implementation to you guys

(and once again, thank's for the package)

spawnia commented 2 years ago

It could be necessary do have arbitrary ordering in the SQL clause, for example ordering by month and ignoring the year:

SUBSTR(created_at, 5, 2) ASC

Adding special support for one specific use case seems to be too narrow of a solution.

guillaumeboussion commented 2 years ago

I get the point, but as you just showed, it could be really useful to handle a custom operator ('SUBSTR', 'ISNULL'...) in the builder, I mean, at least for relations (otherwise, the @builder is enough for this need) If you think of a solution to identify the relation's column to sort by, I take it too :)

spawnia commented 2 years ago

These are not operators though, they are functions. As shown in the example, they can take multiple arguments and are thus not simply addded through some boolean flags. Actually, they can be composed in arbitrarily complex ways - too much power to expose directly to the end user (risk of SQL injection, resource exhaustion).

I could see allowing the definition of custom order expressions in @orderBy and exposing them to the client in some way.

type Query {
    users(
        orderBy: _ @orderBy(expressions: [{ name: "month", builder: "UserBuilder@createdAtMonth" }])
    ): [User!]! @all
}

users(orderBy: { column: MONTH, direction: ASC }) { id }
guillaumeboussion commented 2 years ago

Well, looks great, but for this use I could just do a @builder directive I guess ?

Here is my concrete business example (so that you can understand) : We're selling employee advantages such as discounts in many markets. So, on a brand page, we're displaying all related offers (our partners pay us to have the top position in the query result). Then, we query the Brand type, and join the Offer type as a @belongsTo relation. But, we need to order the main query on the$brand->offer->order attribute.

Of course, I need to first order the non-null value in the order column (so a ISNULL('order') ASC, 'order' ASC) to make them appear this way : 1 2 3 4 null null

And I just can't find a way to @orderBy the offer relation, when querying the Brand type.

Do someone know a simple way to achieve this ? Thanks again :)

spawnia commented 2 years ago

You can always order on the client 😉

My feature proposal could be extended to work on relations too. Adapted to your example:

type Query {
    brands(
        orderBy: _ @orderBy(
          relations: [
            { relation: "offer", expressions: [{ name: "ORDER_NULL_LAST", builder: "Offer@orderNullLast" }])
          ]
        )
    ): [User!]! @all
}

brands(orderBy: { offer: { column: ORDER_NULL_LAST, direction: ASC }}) { ... }
guillaumeboussion commented 2 years ago

Looks great to me! Thanks for your work. I guess in the Offer::orderNullLast(), all I'll have to do is

public function orderNullLast(Builder $builder) : Builder
{
    return $builder->orderByRaw('ISNULL(`order`) ASC, `order` ASC');
}

?

spawnia commented 2 years ago

Looks like we have a good idea of how this could work. Some of the finer details will have to be fleshed out in the implementation:

I am not looking to implement this myself at the moment, but am open to reviewing a PR or doing it as paid work.