Core: [Doctrine][Pagination] Complex OrderBy item must use output walker

Created on 11 Aug 2017  路  7Comments  路  Source: api-platform/core

Most helpful comment

I鈥檝e found that method QueryChecker::hasOrderByOnToManyJoin($queryBuilder, $this->managerRegistry) returns false but it should return true.

It returns false because on QueryChecker.php#L153 $metadata is the joined entity and on QueryChecker.php#L154 $relationship is 'o.joinedEntity'. There is never a field that has a dot in it, and even if there would be, it checks on the wrong metadata.

I did a quick fix with the old code to see whether everything would work and it did. Basically did this:

...
if (false !== strpos($relationship, '.')) {
    list($parentAlias, $association) = explode('.', $relationship);

    $parentMetadata = QueryJoinParser::getClassMetadataFromJoinAlias($parentAlias, $queryBuilder, $managerRegistry);
    if ($parentMetadata->isCollectionValuedAssociation($association)) {
        return true;
    }
} else {
...

This does not handle deeper associations, but I don't know whether that is needed. If @soyuka could look into this, that would be awesome.

All 7 comments

Needs a change in our QueryChecker?

Feel free to continue on https://github.com/api-platform/core/pull/1301

The bigger issue is that it might be unsafe in many cases to not use the output walkers, but we do not know. The only symptom may be incorrect paginated results.

Hello,

I鈥檝e got the same issue with this DQL (Cannot select distinct identifiers from query with LIMIT and ORDER BY on a column from a fetch joined to-many association. Use output walkers) :

SELECT o FROM AppBundle\Entity\Event o 
INNER JOIN o.dates dates_a1 
WHERE o.superEvent IS NULL 
ORDER BY dates_a1.doorTime DESC

This is generated for this request: {{url}}/events?order[dates.doorTime]=DESC

I鈥檝e found that method QueryChecker::hasOrderByOnToManyJoin($queryBuilder, $this->managerRegistry) returns false but it should return true.

I鈥檝e found that method QueryChecker::hasOrderByOnToManyJoin($queryBuilder, $this->managerRegistry) returns false but it should return true.

It returns false because on QueryChecker.php#L153 $metadata is the joined entity and on QueryChecker.php#L154 $relationship is 'o.joinedEntity'. There is never a field that has a dot in it, and even if there would be, it checks on the wrong metadata.

I did a quick fix with the old code to see whether everything would work and it did. Basically did this:

...
if (false !== strpos($relationship, '.')) {
    list($parentAlias, $association) = explode('.', $relationship);

    $parentMetadata = QueryJoinParser::getClassMetadataFromJoinAlias($parentAlias, $queryBuilder, $managerRegistry);
    if ($parentMetadata->isCollectionValuedAssociation($association)) {
        return true;
    }
} else {
...

This does not handle deeper associations, but I don't know whether that is needed. If @soyuka could look into this, that would be awesome.

@rlantingmove4mobile fix works for me.

@ambroisemaupate would you mind sending a PR with your referenced commit?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

rockyweng picture rockyweng  路  3Comments

Tjeerd picture Tjeerd  路  3Comments

dunglas picture dunglas  路  3Comments

CvekCoding picture CvekCoding  路  3Comments

silverbackdan picture silverbackdan  路  3Comments