tpetry / laravel-postgresql-enhanced

Support for many missing PostgreSQL specific features
MIT License
760 stars 32 forks source link

Nulls first/last #31

Closed jaulz closed 2 weeks ago

jaulz commented 2 years ago

I think it would be a great addition if we could order by nulls first/last, i.e. see https://www.postgresql.org/docs/current/sql-select.html#SQL-ORDERBY. Maybe as a new Builder trait? What do you think?

If possible, we should also try to support CursorPaginator since it does not work with raw values at the moment.

tpetry commented 2 years ago

I thought about that too. But i think it is best to add that to Laravel core as it is supported by all DBs.

tpetry commented 2 years ago

Alternatively i could do a trick like with indexes that you can just write orderBy('name NULLS FIRST')

jaulz commented 2 years ago

Ah, that looks like a nice trick as well, thanks! 😊 However, that will not work with cursorPaginate as far as I can see because it requires the direction to be set: https://github.com/laravel/framework/blob/a9bd0ce3b5354fb07861b56263c998819854d5bb/src/Illuminate/Database/Query/Builder.php#L2704-L2717

Unfortunately, it will probably take a while until it lands in the core but I found a fairly simply implementation. The builder needs to override orderBy:

  /**
   * Add an "order by" clause to the query.
   *
   * @param  \Closure|\Illuminate\Database\Query\Builder|\Illuminate\Database\Query\Expression|string  $column
   * @param  string  $direction
   * @param  string  $nulls
   * @return $this
   *
   * @throws \InvalidArgumentException
   */
  public function orderBy($column, $direction = 'asc', $nulls = '')
  {
    parent::orderBy($column, $direction);

    // Retrieve order that was just added and add nulls statement
    $key = $this->unions ? 'unionOrders' : 'orders';
    $order = &$this->{$key}[count($this->{$key}) - 1];
    $order['nulls'] = $nulls;

    return $this;
  }

And the grammar needs this:

    /**
     * Compile the query orders to an array.
     *
     * @param  \Illuminate\Database\Query\Builder  $query
     * @param  array  $orders
     * @return array
     */
    protected function compileOrdersToArray(Builder $query, $orders)
    {
        return collect(parent::compileOrdersToArray($query, $orders))->map(function ($order, $index) use ($orders) {
            return $order . ' ' . $orders[$index]['nulls'];
        })->all();
    }
jaulz commented 2 years ago

Sorry, please ignore the previous code since that does not work with cursorPaginate on subsequent calls.

tpetry commented 1 year ago

@jaulz What was the problem with subsequent cursorPaginate calls? Why didn't your solution work?

jaulz commented 1 year ago

I am not sure anymore what it was exactly but as far as I remember the pagination did not work as expected anymore because it forgets the custom direction parameter on the "second" page.

tpetry commented 2 weeks ago

I've tested everything possible and cursor pagination works with this approach.