whiteoctober / Pagerfanta

Pagination for PHP.
Other
1.59k stars 2 forks source link

Bug in DoctrineDbalAdapter #238

Closed smile1980 closed 5 years ago

smile1980 commented 7 years ago

Hello! I tried to use DoctrineDbalAdapter with driver sqlsrv ( MSSQL ).

If I use sorting in a query builder:

        $connection->createQueryBuilder()
            ->select('t.id')
            ->from('tbl','t')
            ->orderBy('t.order_column','desc');

The adapter create a query:

SELECT COUNT(DISTINCT t.id) AS total_results FROM tbl o ORDER BY t.order_column desc OFFSET 0 ROWS FETCH NEXT 1 ROWS ONLY

For MSSQL such a request is not correct.

To solve the problem, as well as optimize performance, you need to remove the sorting: Add resetQueryPart('orderBy'); method to final query builder modification.

    private function prepareCountQueryBuilder()
    {
        $qb = clone $this->queryBuilder;

        call_user_func( $this->countQueryBuilderModifier, $qb );

        return $qb->resetQueryPart('orderBy');
    }
sampart commented 7 years ago

Hi @smile1980, thanks for contributing.

Apologies if I've misunderstood this, but wouldn't removing the sorting in prepareCountQueryBuilder change the order of the returned results? I don't think we'd want that, so the code would need changing elsewhere to fix this rather than just remove it.

This would be hard for us to replicate here; if you are able to investigate ways to fix this whilst preserving order then we'd welcome a PR!

If this is blocking you in the meantime, you could make your own adapter reusing most of the code from the DoctrineDbalAdapter and use that. I don't know if that helps?

Thanks Sam

sampart commented 5 years ago

Ah, sorry, I hadn't spotted that you were only referring to the count query, so order doesn't matter.