Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-2687

Paginator with ORDER BY not working in MSSQL

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.4
    • Component/s: None
    • Security Level: All
    • Labels:
      None
    • Environment:
      Symfony 2.3, SQL Server 2008 R2

      Description

      This bug report is similar to this one:
      http://www.doctrine-project.org/jira/browse/DDC-2622

      It's decided to make a new bug report to keep things cleaner (commits have already been done on the other report).

      PHP code to test (A symfony 2.3 controller):

      Unable to find source-code formatter for language: php. Available languages are: actionscript, html, java, javascript, none, sql, xhtml, xml
      <?php
      public function testAction() {
      	$em = $this->getDoctrine()->getManager();
      	$query = $em->createQuery("
      		SELECT report, user
      		FROM Report:Report report
      
      		JOIN report.user user
      
      		WHERE user.id = ?1
      		ORDER BY report.created DESC
      	");
      	$query->setMaxResults(10);
      	$query->setParameter(1, 1);
      	$paginator = new Paginator($query, $fetchJoinCollection = true);
      	$result = $paginator->getIterator(); // Trigger querying, using paginator
      	//$result = $query->getResult();
      
      	return [
      		'var' => $result
      	];
      }
      // returning the result to a template debug function
      

      Schema:
      One User to Many Reports

      SQL + ERROR:

      An exception occurred while executing '
      SELECT *
      FROM (
      	SELECT DISTINCT id0
      		,ROW_NUMBER() OVER (
      			ORDER BY r0_.aangemaakt DESC
      			) AS doctrine_rownum
      	FROM (
      		SELECT r0_.id AS id0
      			,r0_.Naam AS Naam1
      			,r0_.Omschrijving AS Omschrijving2
      			,r0_.aangemaakt AS aangemaakt3
      			,r0_.gewijzigd AS gewijzigd4
      			,r0_.verwijderd AS verwijderd5
      			,g1_.username AS username6
      			,g1_.username_canonical AS username_canonical7
      			,g1_.email AS email8
      			,g1_.email_canonical AS email_canonical9
      			,g1_.enabled AS enabled10
      			,g1_.salt AS salt11
      			,g1_.password AS password12
      			,g1_.last_login AS last_login13
      			,g1_.locked AS locked14
      			,g1_.expired AS expired15
      			,g1_.expires_at AS expires_at16
      			,g1_.confirmation_token AS confirmation_token17
      			,g1_.password_requested_at AS password_requested_at18
      			,g1_.roles AS roles19
      			,g1_.credentials_expired AS credentials_expired20
      			,g1_.credentials_expire_at AS credentials_expire_at21
      			,g1_.id AS id22
      		FROM Rapporten r0_ WITH (NOLOCK)
      		INNER JOIN Gebruiker g1_ ON r0_.GebruikerId = g1_.id
      		WHERE (g1_.id = ?)
      			AND (r0_.verwijderd IS NULL)
      		) dctrn_result
      	) AS doctrine_tbl
      WHERE doctrine_rownum BETWEEN 1
      		AND 10
      ' with params [1]:
      
      SQLSTATE[42000]: [Microsoft][SQL Server Native Client 11.0][SQL Server]The multi-part identifier "r0_.aangemaakt" could not be bound. 
      

      FIX:
      Change

      ORDER BY r0_.aangemaakt DESC

      to

      ORDER BY aangemaakt3 DESC

      This fix should only be applied in case of the Paginator scenario, which looks like this:

      Unable to find source-code formatter for language: php. Available languages are: actionscript, html, java, javascript, none, sql, xhtml, xml
      	$paginator = new Paginator($query, $fetchJoinCollection = true);
      	$result = $paginator->getIterator(); // Trigger querying, using paginator
      	//$result = $query->getResult();
      

      When getting a normal result, like this:

      Unable to find source-code formatter for language: php. Available languages are: actionscript, html, java, javascript, none, sql, xhtml, xml
      	//$paginator = new Paginator($query, $fetchJoinCollection = true);
      	//$result = $paginator->getIterator(); // Trigger querying, using paginator
      	$result = $query->getResult();
      

      it works fine without any problems.

      The "normal" result should keep working correctly!

      The SQL produced by the non-paginator result:

      FROM (
      	SELECT r0_.id AS id0
      		,r0_.Naam AS Naam1
      		,r0_.Omschrijving AS Omschrijving2
      		,r0_.aangemaakt AS aangemaakt3
      		,r0_.gewijzigd AS gewijzigd4
      		,r0_.verwijderd AS verwijderd5
      		,g1_.username AS username6
      		,g1_.username_canonical AS username_canonical7
      		,g1_.email AS email8
      		,g1_.email_canonical AS email_canonical9
      		,g1_.enabled AS enabled10
      		,g1_.salt AS salt11
      		,g1_.password AS password12
      		,g1_.last_login AS last_login13
      		,g1_.locked AS locked14
      		,g1_.expired AS expired15
      		,g1_.expires_at AS expires_at16
      		,g1_.confirmation_token AS confirmation_token17
      		,g1_.password_requested_at AS password_requested_at18
      		,g1_.roles AS roles19
      		,g1_.credentials_expired AS credentials_expired20
      		,g1_.credentials_expire_at AS credentials_expire_at21
      		,g1_.id AS id22
      		,r0_.GebruikerId AS GebruikerId23
      		,g1_.group_id AS group_id24
      		,ROW_NUMBER() OVER (
      			ORDER BY r0_.aangemaakt DESC
      			) AS doctrine_rownum
      	FROM Rapporten r0_ WITH (NOLOCK)
      	INNER JOIN Gebruiker g1_ ON r0_.GebruikerId = g1_.id
      	WHERE (g1_.id = ?)
      		AND (r0_.verwijderd IS NULL)
      	) AS doctrine_tbl
      WHERE doctrine_rownum BETWEEN 1
      		AND 10
      

      Notice

      ORDER BY r0_.aangemaakt DESC

      this is correct!

      The difference is that the Paginator wraps the query in in a query with distinct, this is done here:
      https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php#L160

      So far i know two different solutions.

      1.
      Detect the wrapping query in SQLServerPlatform::doModifyLimitQuery with regex. See relevant code snippet below

      Unable to find source-code formatter for language: php. Available languages are: actionscript, html, java, javascript, none, sql, xhtml, xml
      if (preg_match('/SELECT DISTINCT .* FROM \(.*\) dctrn_result/', $query)) {
      		$isWrapped = true;
      } else {
      		$isWrapped = false;
      }
      
      //Find alias for each colum used in ORDER BY
      if ( ! empty($orderbyColumns)) {
      		foreach ($orderbyColumns as $column) {
      
      				if ($isWrapped) {
      						$pattern    = sprintf('/%s\.%s\s+(AS\s+)?([^,\s\)]+)/i', $column['table'], $column['column']);
      						$overColumn = preg_match($pattern, $query, $matches) ? $matches[2] : $column['column'];
      				} else {
      						$pattern    = sprintf('/%s\.(%s)\s*(AS)?\s*([^,\s\)]*)/i', $column['table'], $column['column']);
      						$overColumn = preg_match($pattern, $query, $matches)
      								? ($column['hasTable'] ? $column['table']  . '.' : '') . $column['column'] 
      								: $column['column'];
      				}
      
      				if (isset($column['sort'])) {
      						$overColumn .= ' ' . $column['sort'];
      				}
      
      				$overColumns[] = $overColumn;
      		}
      }
      

      This code would replace:
      https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php#L856-L870
      And takes two lines of codes from this commit:
      https://github.com/doctrine/dbal/blob/5d7bcb6637646eb1daf6d90827233f5562cbda29/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php#L860-L861

      Another solution is that the Paginator should be responsible for notifying the SQLServerPlatform::doModifyLimitQuery method if the query is wrapped or not. In that case, this line of code
      https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php#L171
      should be replaced by

      Unable to find source-code formatter for language: php. Available languages are: actionscript, html, java, javascript, none, sql, xhtml, xml
      $sql, $this->maxResults, $this->firstResult, true
      

      And this line of code:
      https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php#L818
      should be replaced by

      Unable to find source-code formatter for language: php. Available languages are: actionscript, html, java, javascript, none, sql, xhtml, xml
      protected function doModifyLimitQuery($query, $limit, $offset = null, $isWrapped = false)
      

      Of course parts of the code from the previous solution are still needed. But the $isWrapped detection is not done anymore by regex this way.

        Activity

        Hide
        Doctrine Bot added a comment -

        A related Github Pull-Request [GH-789] was closed:
        https://github.com/doctrine/doctrine2/pull/789

        Show
        Doctrine Bot added a comment - A related Github Pull-Request [GH-789] was closed: https://github.com/doctrine/doctrine2/pull/789
        Show
        Marco Pivetta added a comment - Merged: https://github.com/doctrine/dbal/commit/b4bcc18fe0d1b99e37ad51a4a25a04a83ab9d99a
        Hide
        Flip added a comment -

        solved bug, can be closed. See: https://github.com/doctrine/dbal/pull/383

        Show
        Flip added a comment - solved bug, can be closed. See: https://github.com/doctrine/dbal/pull/383
        Hide
        Flip added a comment -

        I ran the full doctrine orm testsuite on a SQL Server 2008 R2 database. A before and after the first suggested bug fix has been applied. Tests results showed no difference at all. This means:
        1. The change does not break any other stuff
        2. There is no test yet to cover this case

        Show
        Flip added a comment - I ran the full doctrine orm testsuite on a SQL Server 2008 R2 database. A before and after the first suggested bug fix has been applied. Tests results showed no difference at all. This means: 1. The change does not break any other stuff 2. There is no test yet to cover this case

          People

          • Assignee:
            Benjamin Eberlei
            Reporter:
            Flip
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: