Skip to content

Loading…

DDC-3029: DISTINCT , ORDER BY AND Limit in SQL Server #3797

Open
doctrinebot opened this Issue · 6 comments

2 participants

@doctrinebot

Jira issue originally created by user bunny1985:

I don't know if I should report it here , becouse error is in SQLServerPlatform.php file.
basicaly Distinct includes ROWNUM()
So if you add distinct in DQL it will be translated to "select distinct ...... , Rownum()" - which is always distinct offcource.

Here is original version

    protected function doModifyLimitQuery($query, $limit, $offset = null)
    {
        if ($limit > 0) {
            if ($offset == 0) {
                $query = preg_replace('/<sup>(SELECT\s(DISTINCT\s)?)/i', '\1TOP ' . $limit . ' ', $query);
            } else {
                $orderby = stristr($query, 'ORDER BY');

                if ( ! $orderby) {
                    $over = 'ORDER BY (SELECT 0)';
                } else {
                    $over = preg*replace('/\"[</sup>,]*\".\"([<sup>,]*)\"/i', '"inner*tbl"."$1"', $orderby);
                }

                // Remove ORDER BY clause from $query
                $query = preg_replace('/\s<ins>ORDER BY(.*)/', '', $query);
                $query = preg_replace('/</sup>SELECT\s/', '', $query);

                $start = $offset </ins> 1;
                $end = $offset + $limit;

                $query = "SELECT * FROM (SELECT ROW*NUMBER() OVER ($over) AS doctrine_rownum, $query) AS doctrine_tbl WHERE doctrine*rownum BETWEEN $start AND $end";
            }
        }

        return $query;
    }

In Attachenment there is a fixed version of this file.

@doctrinebot

Comment created by @ocramius:

Can you provide a failing test case and an expected result as well?

@doctrinebot

Comment created by bunny1985:

Well I'm not working with PHP anymore for now, but i will try:
it's easy and when you look at the code closer you will see the problem:

Test Case: use Query Builder and genearate query with DIstinct and Order BY And Liimit ( top , skip or something ;) )
Expected result would be to get distinct set of data ordered by key and with limit.
Unfortunately - as the result query will be something:
select distinct .... top ....

from .........
SELECT ROWNUMBER() OVER ($over) AS doctrinerownum ... rest of the qurery

so from inner query you will get

doctrine_rownum;column1;column2;column3
1;example;example;example
2;example;example;example
3;example;example;example

I think you can clearly see that each row will be diiffrent. That's why DISTINCT will not work.
What happens next:
At some point doctrine will treat identical entities as one object, so for example if you limit Your query to 10 and all objects will be identical You will get only one Row.

I hope thats clear now :)

And we have a solution for that. It's probably in attached file. I will try to attach final version :)

My team is woried that after updating Doctrine ( symfony ) we will have to face the problem again.

@doctrinebot

Comment created by bunny1985:

well. For some reason i can't attach final version of the file :/
I can paste it as a comment or You can contact me directly and i will send it to You.

@doctrinebot

Comment created by @ocramius:

Ah, I see now what the problem is. ROWNUM() will basically always make the row unique, making DISTINCT useless.

The problem is clear, but it needs an SQL generation test first.

[~bunny1985] can you eventually send the patch as a pull request to https://github.com/doctrine/dbal ? Consider that the code for the SQLServerPlatform changed a lot since 2.4.x

@doctrinebot

Comment created by bunny1985:

Well I could do that in private time, but it can take a while since i have no spare time right now and i would have to prepare some basic PPH dev environment with MSSQL.otherwise i would send untested code which is not a good idea.

@doctrinebot

Comment created by @ocramius:

We all do develop these tools in private time, so nobody is forcing anyone to do anything :-)

@beberlei beberlei was assigned by doctrinebot
@doctrinebot doctrinebot added the Bug label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.