knp-components icon indicating copy to clipboard operation
knp-components copied to clipboard

Doctrine ORM sorting precedence

Open emmanuelballery opened this issue 4 years ago • 1 comments

Hi!

In my projects, I always sort my QueryBuilders by default to have consistent lists throughout my application:

class UserRepository extends EntityRepository
{
    public function findFilter(Filter $filter): QueryBuilder
    {
        $qb = $this->createQueryBuilder('user');

        // my filtering stuff

        return $qb
            ->addOrderBy('user.lastName', 'ASC')
            ->addOrderBy('user.firstName', 'ASC');
    }
}

I'm not always using the Paginator, this is why I'm not using the defaultSort* configurations.

If I try to sort my lists using the Paginator on field user.firstName and direction DESC it will not work.

In fact, if you look at OrderByWalker.php#L77, you'll see that the orderByClause is updated, when possible, instead of prepended all the time.

In my case, the OrderByWalker will update my default orderBy clause on user.firstName to direction DESC. But this clause is not the first one in my query, resulting in a "sub-sorting" after the one on user.lastName ;

In my opinion, sorting by query parameters must always be considered more important than the default QueryBuilder ones. This could be done by always prepending the new clause instead of updating existing ones.

In my code OrderByWalker.php#L76:

            if ($AST->orderByClause) {
                array_unshift($AST->orderByClause->orderByItems, $orderByItem);
            } else {
                $AST->orderByClause = new OrderByClause([$orderByItem]);
            }

This was the initial method back in 2011, but was updated in c114dc7b11ae5a77c87137022b1ac0775d6509cc for reasons I don't get. Do you know the reasons? Do you think we could add a way to opt-out of this "update" mecanism using an option for example?

Thank you!

emmanuelballery avatar Feb 14 '20 09:02 emmanuelballery

I can't tell the reason for mentioned change. Anyway, what I can suggest is to avoid using sorting inside Paginator, even more so if you're already using your own sorting

garak avatar Feb 14 '20 11:02 garak

I'm closing this since the current behaviour is the desired one. Either you use your own sorting or the sorting provided by the library. Using both is not supported.

garak avatar Sep 08 '23 15:09 garak