Paginator fails with $fetchJoinCollection = true and query ordered by a join column. #7018

Open
opened 2026-01-22 15:43:13 +01:00 by admin · 4 comments
Owner

Originally created by @joaquimds on GitHub (Aug 3, 2022).

Bug Report

Q A
BC Break no
Version 2.12.3

Summary

Paginator fails with $fetchJoinCollection = true and query ordered by a join column.

Current behavior

A query generated by the paginator is invalid, and returns an "unknown field" SQL error, because the ORDER BY join_column is not correctly handled.

Imagine a schema as follows:

House Person Pet
id id id
address name person_id
house_id

The paginator works if the ORDER BY uses a normal field, e.g. Person.name - example DQL:

SELECT p from Person p INNER JOIN p.pets pe WITH p.id = pe.person ORDER BY p.name

The SQL looks like:

SELECT DISTINCT id_1 FROM (
  SELECT DISTINCT name_0, id_1 FROM (
    SELECT 
      p0_.name AS name_0, 
      p0_.id AS id_1
    FROM people p0_ INNER JOIN `pets` p1_ 
    ON p0_.id = p1_.person_id
  ) dctrn_result_inner 
  ORDER BY name_0 ASC
) dctrn_result LIMIT 25;

But when the ORDER BY uses a join column, e.g. Person.house:

SELECT p from Person p INNER JOIN p.pets pe WITH p.id = pe.person ORDER BY p.house

The SQL that is generated looks like:

SELECT DISTINCT id_1 FROM (
  SELECT DISTINCT id_1, p0_.house_id FROM (
    SELECT 
      p0_.name AS name_0, 
      p0_.id AS id_1
    FROM people p0_ INNER JOIN `pets` p1_ 
    ON p0_.id = p1_.person_id
  ) dctrn_result_inner 
  ORDER BY p0_.house_id ASC
) dctrn_result LIMIT 25;

house_id is missing in the inner select (because it's a join column, so not in the field names of the class metadata), and it also isn't converted into ORDER BY house_id_1 in the outer query (as name was converted in the previous example).

Relevant functions are SqlWalker::walkSelectClause, which generates the inner SELECT, and LimitSubqueryOutputWalker::recreateInnerSql, which re-applies the ORDER BY clause to the outer query.

How to reproduce

I will work on a reproducable example in my free time, hope to get it to you soon.

Expected behavior

The paginator should be able to sort by join columns. In fact, it does work if $fetchJoinCollection = false.

Originally created by @joaquimds on GitHub (Aug 3, 2022). ### Bug Report | Q | A |------------ | ------ | BC Break | no | Version | 2.12.3 #### Summary Paginator fails with `$fetchJoinCollection = true` and query ordered by a join column. #### Current behavior A query generated by the paginator is invalid, and returns an "unknown field" SQL error, because the `ORDER BY join_column` is not correctly handled. Imagine a schema as follows: | House | Person | Pet |------------ | --------- | -------- | id | id | id | address | name | person_id | | house_id | The paginator works if the `ORDER BY` uses a normal field, e.g. `Person.name` - example DQL: ``` SELECT p from Person p INNER JOIN p.pets pe WITH p.id = pe.person ORDER BY p.name ``` The SQL looks like: ``` SELECT DISTINCT id_1 FROM ( SELECT DISTINCT name_0, id_1 FROM ( SELECT p0_.name AS name_0, p0_.id AS id_1 FROM people p0_ INNER JOIN `pets` p1_ ON p0_.id = p1_.person_id ) dctrn_result_inner ORDER BY name_0 ASC ) dctrn_result LIMIT 25; ``` But when the `ORDER BY` uses a join column, e.g. `Person.house`: ``` SELECT p from Person p INNER JOIN p.pets pe WITH p.id = pe.person ORDER BY p.house ``` The SQL that is generated looks like: ``` SELECT DISTINCT id_1 FROM ( SELECT DISTINCT id_1, p0_.house_id FROM ( SELECT p0_.name AS name_0, p0_.id AS id_1 FROM people p0_ INNER JOIN `pets` p1_ ON p0_.id = p1_.person_id ) dctrn_result_inner ORDER BY p0_.house_id ASC ) dctrn_result LIMIT 25; ``` `house_id` is missing in the inner select (because it's a join column, so not in the field names of the class metadata), and it also isn't converted into `ORDER BY house_id_1` in the outer query (as `name` was converted in the previous example). Relevant functions are `SqlWalker::walkSelectClause`, which generates the inner SELECT, and `LimitSubqueryOutputWalker::recreateInnerSql`, which re-applies the `ORDER BY` clause to the outer query. #### How to reproduce I will work on a reproducable example in my free time, hope to get it to you soon. #### Expected behavior The paginator should be able to sort by join columns. In fact, it does work if `$fetchJoinCollection = false`.
Author
Owner

@joaquimds commented on GitHub (Aug 3, 2022):

I can get around this issue by manually telling Doctrine to use the standard SqlWalker instead of LimitSubqueryOutputWalker:

 $paginator
    ->getQuery()
    ->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, \Doctrine\ORM\Query\SqlWalker::class);

This seems to work, and the pagination still works correctly, even if there are multiple child entities for each root entity (which I believe is the reason $fetchJoinCollection = true is needed). Which raises the question: what is the purpose of LimitSubqueryOutputWalker? I think I'm missing something.

@joaquimds commented on GitHub (Aug 3, 2022): I can get around this issue by manually telling Doctrine to use the standard `SqlWalker` instead of `LimitSubqueryOutputWalker`: ``` $paginator ->getQuery() ->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, \Doctrine\ORM\Query\SqlWalker::class); ``` This seems to work, and the pagination still works correctly, even if there are multiple child entities for each root entity (which I believe is the reason `$fetchJoinCollection = true` is needed). Which raises the question: what is the purpose of `LimitSubqueryOutputWalker`? I think I'm missing something.
Author
Owner

@joaquimds commented on GitHub (Aug 10, 2022):

I have created a unit test for this and a "fix", but I can't push a branch to make a PR. The fix I have done feels dirty so I think someone who understands Doctrine better than I do should rework it.

The test is:

tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php:

public function testLimitSubqueryWithOrderByJoinColumn(): void
{
    $query = $this->entityManager->createQuery(
        'SELECT b FROM Doctrine\Tests\ORM\Tools\Pagination\BlogPost b JOIN b.author a ORDER BY b.author ASC'
    );

    $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, LimitSubqueryOutputWalker::class);

    self::assertSame(
        'SELECT DISTINCT id_0 FROM (SELECT DISTINCT id_0, author_id_1 FROM (SELECT b0_.id AS id_0, b0_.author_id AS author_id_1, b0_.category_id AS category_id_2 FROM BlogPost b0_ INNER JOIN Author a1_ ON b0_.author_id = a1_.id) dctrn_result_inner ORDER BY author_id_1 ASC) dctrn_result',
        $query->getSQL()
    );
}

The fix is:

storage/code/doctrine-orm/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php

private function generateSqlAliasReplacements(): array
{
    $aliasMap = $searchPatterns = $replacements = $metadataList = [];

    // Generate DQL alias -> SQL table alias mapping
    foreach (array_keys($this->rsm->aliasMap) as $dqlAlias) {
        $metadataList[$dqlAlias] = $class = $this->getMetadataForDqlAlias($dqlAlias);
        $aliasMap[$dqlAlias]     = $this->getSQLTableAlias($class->getTableName(), $dqlAlias);
    }

    // Generate search patterns for each field's path expression in the order by clause
    $fieldMappings = array_merge($this->rsm->fieldMappings, $this->rsm->metaMappings);
    foreach ($fieldMappings as $fieldAlias => $fieldName) {
        $dqlAliasForFieldAlias = $this->rsm->columnOwnerMap[$fieldAlias];
        $class                 = $metadataList[$dqlAliasForFieldAlias];

        $fieldMapping = null;
        $columnName = null;
        if (isset($class->fieldMappings[$fieldName])) {
            $fieldMapping = $class->fieldMappings[$fieldName];
            // Get the proper column name as will appear in the select list
            $columnName = $this->quoteStrategy->getColumnName(
                $fieldName,
                $metadataList[$dqlAliasForFieldAlias],
                $this->em->getConnection()->getDatabasePlatform()
            );
        } else {
            foreach ($class->associationMappings as $name => $assoc) {
                foreach ($assoc['joinColumns'] as $joinColumn) {
                    if ($joinColumn['name'] === $fieldName) {
                        $fieldMapping = $assoc;
                        // Get the proper column name as will appear in the select list
                        $columnName = $this->quoteStrategy->getJoinColumnName(
                            $joinColumn,
                            $metadataList[$dqlAliasForFieldAlias],
                            $this->em->getConnection()->getDatabasePlatform()
                        );
                    }
                }
            }
        }

        // If the field is from a joined child table, we won't be ordering on it.
        if (!$fieldMapping) {
            continue;
        }

        // Get the SQL table alias for the entity and field
        $sqlTableAliasForFieldAlias = $aliasMap[$dqlAliasForFieldAlias];

        if (isset($fieldMapping['declared']) && $fieldMapping['declared'] !== $class->name) {
            // Field was declared in a parent class, so we need to get the proper SQL table alias
            // for the joined parent table.
            $otherClassMetadata = $this->em->getClassMetadata($fieldMapping['declared']);

            if (! $otherClassMetadata->isMappedSuperclass) {
                $sqlTableAliasForFieldAlias = $this->getSQLTableAlias($otherClassMetadata->getTableName(), $dqlAliasForFieldAlias);
            }
        }

        // Compose search and replace patterns
        $searchPatterns[] = sprintf(self::ORDER_BY_PATH_EXPRESSION, $sqlTableAliasForFieldAlias, $columnName);
        $replacements[]   = $fieldAlias;
    }

    return [$searchPatterns, $replacements];
}
@joaquimds commented on GitHub (Aug 10, 2022): I have created a unit test for this and a "fix", but I can't push a branch to make a PR. The fix I have done feels dirty so I think someone who understands Doctrine better than I do should rework it. The test is: `tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php`: ```php public function testLimitSubqueryWithOrderByJoinColumn(): void { $query = $this->entityManager->createQuery( 'SELECT b FROM Doctrine\Tests\ORM\Tools\Pagination\BlogPost b JOIN b.author a ORDER BY b.author ASC' ); $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, LimitSubqueryOutputWalker::class); self::assertSame( 'SELECT DISTINCT id_0 FROM (SELECT DISTINCT id_0, author_id_1 FROM (SELECT b0_.id AS id_0, b0_.author_id AS author_id_1, b0_.category_id AS category_id_2 FROM BlogPost b0_ INNER JOIN Author a1_ ON b0_.author_id = a1_.id) dctrn_result_inner ORDER BY author_id_1 ASC) dctrn_result', $query->getSQL() ); } ``` The fix is: `storage/code/doctrine-orm/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php` ```php private function generateSqlAliasReplacements(): array { $aliasMap = $searchPatterns = $replacements = $metadataList = []; // Generate DQL alias -> SQL table alias mapping foreach (array_keys($this->rsm->aliasMap) as $dqlAlias) { $metadataList[$dqlAlias] = $class = $this->getMetadataForDqlAlias($dqlAlias); $aliasMap[$dqlAlias] = $this->getSQLTableAlias($class->getTableName(), $dqlAlias); } // Generate search patterns for each field's path expression in the order by clause $fieldMappings = array_merge($this->rsm->fieldMappings, $this->rsm->metaMappings); foreach ($fieldMappings as $fieldAlias => $fieldName) { $dqlAliasForFieldAlias = $this->rsm->columnOwnerMap[$fieldAlias]; $class = $metadataList[$dqlAliasForFieldAlias]; $fieldMapping = null; $columnName = null; if (isset($class->fieldMappings[$fieldName])) { $fieldMapping = $class->fieldMappings[$fieldName]; // Get the proper column name as will appear in the select list $columnName = $this->quoteStrategy->getColumnName( $fieldName, $metadataList[$dqlAliasForFieldAlias], $this->em->getConnection()->getDatabasePlatform() ); } else { foreach ($class->associationMappings as $name => $assoc) { foreach ($assoc['joinColumns'] as $joinColumn) { if ($joinColumn['name'] === $fieldName) { $fieldMapping = $assoc; // Get the proper column name as will appear in the select list $columnName = $this->quoteStrategy->getJoinColumnName( $joinColumn, $metadataList[$dqlAliasForFieldAlias], $this->em->getConnection()->getDatabasePlatform() ); } } } } // If the field is from a joined child table, we won't be ordering on it. if (!$fieldMapping) { continue; } // Get the SQL table alias for the entity and field $sqlTableAliasForFieldAlias = $aliasMap[$dqlAliasForFieldAlias]; if (isset($fieldMapping['declared']) && $fieldMapping['declared'] !== $class->name) { // Field was declared in a parent class, so we need to get the proper SQL table alias // for the joined parent table. $otherClassMetadata = $this->em->getClassMetadata($fieldMapping['declared']); if (! $otherClassMetadata->isMappedSuperclass) { $sqlTableAliasForFieldAlias = $this->getSQLTableAlias($otherClassMetadata->getTableName(), $dqlAliasForFieldAlias); } } // Compose search and replace patterns $searchPatterns[] = sprintf(self::ORDER_BY_PATH_EXPRESSION, $sqlTableAliasForFieldAlias, $columnName); $replacements[] = $fieldAlias; } return [$searchPatterns, $replacements]; } ```
Author
Owner

@GlucNAc commented on GitHub (Oct 21, 2022):

I do reproduce this and it is clearly a breaking change bug as $fetchJoinCollection = true is the default value for the Paginator.

@joaquimds I do have rigth to create a PR then I think you too. Even if the code is not clean, it would be the beggining of the fix :)

@GlucNAc commented on GitHub (Oct 21, 2022): I do reproduce this and it is clearly a breaking change bug as `$fetchJoinCollection = true` is the default value for the Paginator. @joaquimds I do have rigth to create a PR then I think you too. Even if the code is not clean, it would be the beggining of the fix :)
Author
Owner

@petski commented on GitHub (Oct 23, 2024):

Hello from 2024 :). Found this report while investigating an issue I've came across.

The relevant part of the Query I supply to the Paginator looks like this:

->select('x')
->addSelect('ST_Distance_Sphere(x.geographyPoint, :referenceGeographyPoint) AS sphericalDistanceInMeter')
->setParameter('referenceGeographyPoint', $referenceGeographyPoint, 'point')
...
->orderBy('sphericalDistanceInMeter', 'ASC')

The point type is doing something like return sprintf('ST_GeomFromText(%s)', $sqlExpr) in its convertToDatabaseValue.

The COUNT-SQLquery works as expected.

The DISTINCT ... LIMIT-SQLquery results in an invalid SQL statement: The SQLquery results in ST_Distance_Sphere(s0_.geography_point, ?) instead of a ST_Distance_Sphere(s0_.geography_point, ST_GeomFromText(?)). The SQLquery is invalid because of this, so resulting in a SQL exception.

If, as a test, I remove the orderBy from the query, then all works as expected. All three (count, distinct-limit, where-in) queries are valid, apart from the ordering of course.

I think "my issue" is strongly related to this report.

Setting $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, \Doctrine\ORM\Query\SqlWalker::class); is a working work-around.

Replacing LimitSubqueryOutputWalker::generateSqlAliasReplacements() with the version supplied by @joaquimds does NOT resolve my issue though.

@petski commented on GitHub (Oct 23, 2024): Hello from 2024 :). Found this report while investigating an issue I've came across. The relevant part of the `Query` I supply to the `Paginator` looks like this: ``` ->select('x') ->addSelect('ST_Distance_Sphere(x.geographyPoint, :referenceGeographyPoint) AS sphericalDistanceInMeter') ->setParameter('referenceGeographyPoint', $referenceGeographyPoint, 'point') ... ->orderBy('sphericalDistanceInMeter', 'ASC') ``` The `point` type is doing something like `return sprintf('ST_GeomFromText(%s)', $sqlExpr)` in its `convertToDatabaseValue`. The `COUNT`-SQLquery works as expected. The `DISTINCT ... LIMIT`-SQLquery results in an invalid SQL statement: The SQLquery results in `ST_Distance_Sphere(s0_.geography_point, ?)` instead of a `ST_Distance_Sphere(s0_.geography_point, ST_GeomFromText(?))`. The SQLquery is invalid because of this, so resulting in a SQL exception. If, as a test, I remove the `orderBy` from the query, then all works as expected. All three (count, distinct-limit, where-in) queries are valid, apart from the ordering of course. I think "my issue" is strongly related to this report. Setting `$query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, \Doctrine\ORM\Query\SqlWalker::class);` is a working work-around. Replacing `LimitSubqueryOutputWalker::generateSqlAliasReplacements()` with the version supplied by @joaquimds does NOT resolve my issue though.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#7018