Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-1785

Paginator problem with SQL Server around DISTINCT keyword.

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.1
    • Fix Version/s: None
    • Component/s: None
    • Security Level: All
    • Labels:
      None

      Description

      PDOException: SQLSTATE[42000]: [Microsoft][SQL Server Native Client 10.0][SQL Server]Incorrect syntax near the keyword 'DISTINCT'. (uncaught exception)

        Activity

        Hide
        Craig Mason added a comment - - edited

        There are four major issues with this:

        1: SQLServerPlatform.php modifies the query to prepend 'SELECT ROW_NUMBER() OVER ($over)', which is inserted before the DISTINCT keyword.

        2: The order needs to be placed inside the OVER($over) block. At this point, the regex is using the exact column name rather than the alias, so the outer query cannot ORDER.

        3: The DISTINCT queries select only the ID columns - as OVER() required the sort column to be available in the outer query, IDs alone will not work.

        4: SQL Server cannot DISTINCT on TEXT columns. 2005,2008 and 2012 recommend using VARCHAR(MAX) instead, which does support it. That doesn't help us with 2003. We work around that with a custom TEXT type that casts as varchar.

        Incidentally, 2012 supports LIMIT, which gets rid of this issue altogether.

        Edit: Added #3

        Show
        Craig Mason added a comment - - edited There are four major issues with this: 1: SQLServerPlatform.php modifies the query to prepend 'SELECT ROW_NUMBER() OVER ($over)', which is inserted before the DISTINCT keyword. 2: The order needs to be placed inside the OVER($over) block. At this point, the regex is using the exact column name rather than the alias, so the outer query cannot ORDER. 3: The DISTINCT queries select only the ID columns - as OVER() required the sort column to be available in the outer query, IDs alone will not work. 4: SQL Server cannot DISTINCT on TEXT columns. 2005,2008 and 2012 recommend using VARCHAR(MAX) instead, which does support it. That doesn't help us with 2003. We work around that with a custom TEXT type that casts as varchar. Incidentally, 2012 supports LIMIT, which gets rid of this issue altogether. Edit: Added #3
        Hide
        Craig Mason added a comment -

        I have a (very hacky) implementation working that uses regexes to correct the query so that it will execute. This also required modification in the ORM paginator, to select all columns instead of just IDs.

        https://github.com/CraigMason/dbal/commit/4ecd018c73e387904f78d81f1d327e34e905c5f1
        https://github.com/CraigMason/doctrine2/commit/b416d3b2a38495e4435bde872b19fec371fe5657

        This is certainly not a patch - more guidance.

        One interesting point... I had to wrap the whole query in a second SELECT *, as the WHERE IN confusingly returns non-distinct rows when part of the first inner query. No idea why this happens, but moving it out one layer makes it operate correctly.

        Show
        Craig Mason added a comment - I have a (very hacky) implementation working that uses regexes to correct the query so that it will execute. This also required modification in the ORM paginator, to select all columns instead of just IDs. https://github.com/CraigMason/dbal/commit/4ecd018c73e387904f78d81f1d327e34e905c5f1 https://github.com/CraigMason/doctrine2/commit/b416d3b2a38495e4435bde872b19fec371fe5657 This is certainly not a patch - more guidance. One interesting point... I had to wrap the whole query in a second SELECT *, as the WHERE IN confusingly returns non-distinct rows when part of the first inner query. No idea why this happens, but moving it out one layer makes it operate correctly.
        Hide
        Craig Mason added a comment -

        Updated, view all commits for this experimental branch here:

        https://github.com/CraigMason/dbal/commits/mssql-distinct

        Show
        Craig Mason added a comment - Updated, view all commits for this experimental branch here: https://github.com/CraigMason/dbal/commits/mssql-distinct
        Hide
        Craig Mason added a comment -

        This got waaaay too messy with regex alone due to the complicated nesting. As such, I have written the basis of a new SqlWalker class which can be used to create DISTINCT queries based on the root identifiers. It's not proper DISTINCT support, but it's a step forward.

        https://github.com/CraigMason/DoctrineSqlServerExtensions

        I've also added a Paginator (which was the original issue I had!)

        The current SqlWalker always sticks the ORDER BY on the end of the query, which just doesn't work properly with SqlServer. Is a vendor-specific walker breaking the DQL abstraction? Should this type of code be on the Platform object in the DBAL?

        Anyway, this repo fixes our immediate problem, and it would be good to revisit this in a wider context. Hopefully we can get some good SQL server support - there are plenty of other issues to deal with (UTF-8/UCS2, nvarchar etc)

        Show
        Craig Mason added a comment - This got waaaay too messy with regex alone due to the complicated nesting. As such, I have written the basis of a new SqlWalker class which can be used to create DISTINCT queries based on the root identifiers. It's not proper DISTINCT support, but it's a step forward. https://github.com/CraigMason/DoctrineSqlServerExtensions I've also added a Paginator (which was the original issue I had!) The current SqlWalker always sticks the ORDER BY on the end of the query, which just doesn't work properly with SqlServer. Is a vendor-specific walker breaking the DQL abstraction? Should this type of code be on the Platform object in the DBAL? Anyway, this repo fixes our immediate problem, and it would be good to revisit this in a wider context. Hopefully we can get some good SQL server support - there are plenty of other issues to deal with (UTF-8/UCS2, nvarchar etc)
        Hide
        Benjamin Eberlei added a comment -

        Craig Mason We don't have an SQL Server expert on the team, so if you want really good support you should join and help us with it.

        Show
        Benjamin Eberlei added a comment - Craig Mason We don't have an SQL Server expert on the team, so if you want really good support you should join and help us with it.

          People

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

            Dates

            • Created:
              Updated: