{"action":"created","comment":{"url":"https://api.github.com/repos/doctrine/dbal/pulls/comments/47673499","id":47673499,"diff_hunk":"@@ -1182,77 +1182,113 @@ protected function doModifyLimitQuery($query, $limit, $offset = null)\n \n         $start   = $offset + 1;\n         $end     = $offset + $limit;\n-        $orderBy = stristr($query, 'ORDER BY');\n \n-        //Remove ORDER BY from $query (including nested parentheses in order by list).\n-        $query = preg_replace('/\\s+ORDER\\s+BY\\s+([^()]+|\\((?:(?:(?>[^()]+)|(?R))*)\\))+/i', '', $query);\n-\n-        $format  = 'SELECT * FROM (%s) AS doctrine_tbl WHERE doctrine_rownum BETWEEN %d AND %d ORDER BY doctrine_rownum';\n-\n-        // Pattern to match \"main\" SELECT ... FROM clause (including nested parentheses in select list).\n-        $selectFromPattern = '/^(\\s*SELECT\\s+(?:(.*)(?![^(]*\\))))\\sFROM\\s/i';\n-\n-        if ( ! $orderBy) {\n-            //Replace only \"main\" FROM with OVER to prevent changing FROM also in subqueries.\n-            $query = preg_replace(\n-                $selectFromPattern,\n-                '$1, ROW_NUMBER() OVER (ORDER BY (SELECT 0)) AS doctrine_rownum FROM ',\n-                $query,\n-                1\n-            );\n-\n-            return sprintf($format, $query, $start, $end);\n+        // We'll find a SELECT or SELECT distinct and prepend TOP n to it\n+        // Even if the TOP n is very large, the use of a CTE will\n+        // allow the SQL Server query planner to optimize it so it doesn't\n+        // actually scan the entire range covered by the TOP clause.\n+        $selectPattern = '/^(\\s*SELECT\\s+(?:DISTINCT)?\\s*)(.*)$/i';\n+        $replacePattern = sprintf('$1%s $2', \"TOP $end\");\n+        $query = preg_replace($selectPattern, $replacePattern, $query);\n+\n+        if (stristr($query, \"ORDER BY\")) {\n+            // Inner order by is not valid in SQL Server for our purposes\n+            // unless it's in a TOP N subquery.\n+            $query = $this->scrubInnerOrderBy($query);\n         }\n \n-        //Clear ORDER BY\n-        $orderBy        = preg_replace('/ORDER\\s+BY\\s+(.*)/i', '$1', $orderBy);\n-        $orderByParts   = explode(',', $orderBy);\n-        $orderByColumns = array();\n+        // Build a new limited query around the original, using a CTE\n+        return sprintf(\n+            \"WITH dctrn_cte AS (%s) \"\n+            . \"SELECT * FROM (\"\n+            . \"SELECT *, ROW_NUMBER() OVER (ORDER BY (SELECT 0)) AS doctrine_rownum FROM dctrn_cte\"\n+            . \") AS doctrine_tbl \"\n+            . \"WHERE doctrine_rownum BETWEEN %d AND %d ORDER BY doctrine_rownum ASC\",\n+            $query,\n+            $start,\n+            $end\n+        );\n+    }\n \n-        //Split ORDER BY into parts\n-        foreach ($orderByParts as &$part) {\n+    /**\n+     * Remove ORDER BY clauses in subqueries - they're not supported by SQL Server.\n+     * Caveat: will leave ORDER BY in TOP N subqueries.\n+     *\n+     * @param $query\n+     * @return string\n+     *\n+     */\n+    private function scrubInnerOrderBy($query) {\n+        $count = substr_count(strtoupper($query), \"ORDER BY\");\n+        $offset = 0;\n+        while ($count-- > 0) {\n+            $qLen = strlen($query);\n+            $orderByPos = stripos($query, \" ORDER BY\", $offset);\n+            $parenCount = 0;\n+            $currentPosition = $orderByPos;\n+\n+            while ($parenCount >= 0 && $currentPosition < $qLen) {\n+                if ($query[$currentPosition] == '(') {\n+                    $parenCount++;\n+                } elseif ($query[$currentPosition] == ')') {\n+                    $parenCount--;\n+                }\n \n-            if (preg_match('/(([^\\s]*)\\.)?([^\\.\\s]*)\\s*(ASC|DESC)?/i', trim($part), $matches)) {\n-                $orderByColumns[] = array(\n-                    'column'    => $matches[3],\n-                    'hasTable'  => ( ! empty($matches[2])),\n-                    'sort'      => isset($matches[4]) ? $matches[4] : null,\n-                    'table'     => empty($matches[2]) ? '[^\\.\\s]*' : $matches[2]\n-                );\n+                $currentPosition++;\n             }\n-        }\n-\n-        $isWrapped = (preg_match('/SELECT DISTINCT .* FROM \\(.*\\) dctrn_result/', $query)) ? true : false;\n \n-        $overColumns = array();\n-\n-        //Find alias for each column used in ORDER BY\n-        if ( ! empty($orderByColumns)) {\n-            foreach ($orderByColumns as $column) {\n-                $pattern = sprintf('/%s\\.%s\\s+(?:AS\\s+)?([^,\\s)]+)/i', $column['table'], $column['column']);\n+            if ($this->isOrderByInTopNSubquery($query, $orderByPos)) {\n+                // If the order by clause is in a TOP N subquery, do not remove\n+                // it and continue iteration from the current position.\n+                $offset = $currentPosition;\n+                continue;\n+            }\n \n-                if ($isWrapped) {\n-                    $overColumn = preg_match($pattern, $query, $matches)\n-                        ? $matches[1] : '';\n-                } else {\n-                    $overColumn = preg_match($pattern, $query, $matches)\n-                        ? ($column['hasTable'] ? $column['table']  . '.' : '') . $column['column']\n-                        : $column['column'];\n-                }\n+            if ($currentPosition < $qLen - 1) {\n+                $query = substr($query, 0, $orderByPos) . substr($query, $currentPosition - 1);\n+                $offset = $orderByPos;\n+            }\n+        }\n+        return $query;","path":"lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php","position":111,"original_position":120,"commit_id":"a22c1bf4a75cd27ec954d9e6675ab4bd5b5d8a42","original_commit_id":"70e5d371e9621ccc711fbaf630df96d4c225ddd4","user":{"login":"zeroedin-bill","id":4266849,"avatar_url":"https://avatars.githubusercontent.com/u/4266849?v=3","gravatar_id":"","url":"https://api.github.com/users/zeroedin-bill","html_url":"https://github.com/zeroedin-bill","followers_url":"https://api.github.com/users/zeroedin-bill/followers","following_url":"https://api.github.com/users/zeroedin-bill/following{/other_user}","gists_url":"https://api.github.com/users/zeroedin-bill/gists{/gist_id}","starred_url":"https://api.github.com/users/zeroedin-bill/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/zeroedin-bill/subscriptions","organizations_url":"https://api.github.com/users/zeroedin-bill/orgs","repos_url":"https://api.github.com/users/zeroedin-bill/repos","events_url":"https://api.github.com/users/zeroedin-bill/events{/privacy}","received_events_url":"https://api.github.com/users/zeroedin-bill/received_events","type":"User","site_admin":false},"body":"There's a nl before already","created_at":"2015-12-15T18:05:45Z","updated_at":"2015-12-15T18:05:45Z","html_url":"https://github.com/doctrine/dbal/pull/818#discussion_r47673499","pull_request_url":"https://api.github.com/repos/doctrine/dbal/pulls/818","_links":{"self":{"href":"https://api.github.com/repos/doctrine/dbal/pulls/comments/47673499"},"html":{"href":"https://github.com/doctrine/dbal/pull/818#discussion_r47673499"},"pull_request":{"href":"https://api.github.com/repos/doctrine/dbal/pulls/818"}}},"pull_request":{"url":"https://api.github.com/repos/doctrine/dbal/pulls/818","id":31387668,"html_url":"https://github.com/doctrine/dbal/pull/818","diff_url":"https://github.com/doctrine/dbal/pull/818.diff","patch_url":"https://github.com/doctrine/dbal/pull/818.patch","issue_url":"https://api.github.com/repos/doctrine/dbal/issues/818","number":818,"state":"open","locked":false,"title":"Rebuild SQLServerPlatform::doModifyLimitQuery again to use a CTE","user":{"login":"zeroedin-bill","id":4266849,"avatar_url":"https://avatars.githubusercontent.com/u/4266849?v=3","gravatar_id":"","url":"https://api.github.com/users/zeroedin-bill","html_url":"https://github.com/zeroedin-bill","followers_url":"https://api.github.com/users/zeroedin-bill/followers","following_url":"https://api.github.com/users/zeroedin-bill/following{/other_user}","gists_url":"https://api.github.com/users/zeroedin-bill/gists{/gist_id}","starred_url":"https://api.github.com/users/zeroedin-bill/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/zeroedin-bill/subscriptions","organizations_url":"https://api.github.com/users/zeroedin-bill/orgs","repos_url":"https://api.github.com/users/zeroedin-bill/repos","events_url":"https://api.github.com/users/zeroedin-bill/events{/privacy}","received_events_url":"https://api.github.com/users/zeroedin-bill/received_events","type":"User","site_admin":false},"body":"Also only uses 1 regex.\r\n\r\nThis method is vastly simpler and more reliable than the old method of parsing a whole lot of stuff.\r\n\r\nA caveat: if a query is passed that contains a subquery with an ORDER BY clause, the inner ORDER BY clause will be dropped. SQL server doesn't support using ORDER BY inside subqueries. In the future I think we should consider throwing an exception when these are found.\r\n\r\n~~For now, hold off on merging this. There is a PR open for the ORM Paginator that prevents it from sending queries with multiple ORDER BY clauses. The Paginator is the only place in the ORM where this occurs. Until that PR is merged, this one should probably stay open.~~\r\n\r\nI'll be adding a set of functional tests for the Paginator later this week.","created_at":"2015-03-17T23:17:35Z","updated_at":"2015-12-15T18:05:46Z","closed_at":null,"merged_at":null,"merge_commit_sha":"04ac624a4311db88b7cba73d488e89963fa3c605","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/doctrine/dbal/pulls/818/commits","review_comments_url":"https://api.github.com/repos/doctrine/dbal/pulls/818/comments","review_comment_url":"https://api.github.com/repos/doctrine/dbal/pulls/comments{/number}","comments_url":"https://api.github.com/repos/doctrine/dbal/issues/818/comments","statuses_url":"https://api.github.com/repos/doctrine/dbal/statuses/a22c1bf4a75cd27ec954d9e6675ab4bd5b5d8a42","head":{"label":"zeroedin-bill:sqlserverlimitfix-2","ref":"sqlserverlimitfix-2","sha":"a22c1bf4a75cd27ec954d9e6675ab4bd5b5d8a42","user":{"login":"zeroedin-bill","id":4266849,"avatar_url":"https://avatars.githubusercontent.com/u/4266849?v=3","gravatar_id":"","url":"https://api.github.com/users/zeroedin-bill","html_url":"https://github.com/zeroedin-bill","followers_url":"https://api.github.com/users/zeroedin-bill/followers","following_url":"https://api.github.com/users/zeroedin-bill/following{/other_user}","gists_url":"https://api.github.com/users/zeroedin-bill/gists{/gist_id}","starred_url":"https://api.github.com/users/zeroedin-bill/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/zeroedin-bill/subscriptions","organizations_url":"https://api.github.com/users/zeroedin-bill/orgs","repos_url":"https://api.github.com/users/zeroedin-bill/repos","events_url":"https://api.github.com/users/zeroedin-bill/events{/privacy}","received_events_url":"https://api.github.com/users/zeroedin-bill/received_events","type":"User","site_admin":false},"repo":{"id":13302396,"name":"dbal","full_name":"zeroedin-bill/dbal","owner":{"login":"zeroedin-bill","id":4266849,"avatar_url":"https://avatars.githubusercontent.com/u/4266849?v=3","gravatar_id":"","url":"https://api.github.com/users/zeroedin-bill","html_url":"https://github.com/zeroedin-bill","followers_url":"https://api.github.com/users/zeroedin-bill/followers","following_url":"https://api.github.com/users/zeroedin-bill/following{/other_user}","gists_url":"https://api.github.com/users/zeroedin-bill/gists{/gist_id}","starred_url":"https://api.github.com/users/zeroedin-bill/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/zeroedin-bill/subscriptions","organizations_url":"https://api.github.com/users/zeroedin-bill/orgs","repos_url":"https://api.github.com/users/zeroedin-bill/repos","events_url":"https://api.github.com/users/zeroedin-bill/events{/privacy}","received_events_url":"https://api.github.com/users/zeroedin-bill/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/zeroedin-bill/dbal","description":"Doctrine Database Abstraction Layer","fork":true,"url":"https://api.github.com/repos/zeroedin-bill/dbal","forks_url":"https://api.github.com/repos/zeroedin-bill/dbal/forks","keys_url":"https://api.github.com/repos/zeroedin-bill/dbal/keys{/key_id}","collaborators_url":"https://api.github.com/repos/zeroedin-bill/dbal/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/zeroedin-bill/dbal/teams","hooks_url":"https://api.github.com/repos/zeroedin-bill/dbal/hooks","issue_events_url":"https://api.github.com/repos/zeroedin-bill/dbal/issues/events{/number}","events_url":"https://api.github.com/repos/zeroedin-bill/dbal/events","assignees_url":"https://api.github.com/repos/zeroedin-bill/dbal/assignees{/user}","branches_url":"https://api.github.com/repos/zeroedin-bill/dbal/branches{/branch}","tags_url":"https://api.github.com/repos/zeroedin-bill/dbal/tags","blobs_url":"https://api.github.com/repos/zeroedin-bill/dbal/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/zeroedin-bill/dbal/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/zeroedin-bill/dbal/git/refs{/sha}","trees_url":"https://api.github.com/repos/zeroedin-bill/dbal/git/trees{/sha}","statuses_url":"https://api.github.com/repos/zeroedin-bill/dbal/statuses/{sha}","languages_url":"https://api.github.com/repos/zeroedin-bill/dbal/languages","stargazers_url":"https://api.github.com/repos/zeroedin-bill/dbal/stargazers","contributors_url":"https://api.github.com/repos/zeroedin-bill/dbal/contributors","subscribers_url":"https://api.github.com/repos/zeroedin-bill/dbal/subscribers","subscription_url":"https://api.github.com/repos/zeroedin-bill/dbal/subscription","commits_url":"https://api.github.com/repos/zeroedin-bill/dbal/commits{/sha}","git_commits_url":"https://api.github.com/repos/zeroedin-bill/dbal/git/commits{/sha}","comments_url":"https://api.github.com/repos/zeroedin-bill/dbal/comments{/number}","issue_comment_url":"https://api.github.com/repos/zeroedin-bill/dbal/issues/comments{/number}","contents_url":"https://api.github.com/repos/zeroedin-bill/dbal/contents/{+path}","compare_url":"https://api.github.com/repos/zeroedin-bill/dbal/compare/{base}...{head}","merges_url":"https://api.github.com/repos/zeroedin-bill/dbal/merges","archive_url":"https://api.github.com/repos/zeroedin-bill/dbal/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/zeroedin-bill/dbal/downloads","issues_url":"https://api.github.com/repos/zeroedin-bill/dbal/issues{/number}","pulls_url":"https://api.github.com/repos/zeroedin-bill/dbal/pulls{/number}","milestones_url":"https://api.github.com/repos/zeroedin-bill/dbal/milestones{/number}","notifications_url":"https://api.github.com/repos/zeroedin-bill/dbal/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/zeroedin-bill/dbal/labels{/name}","releases_url":"https://api.github.com/repos/zeroedin-bill/dbal/releases{/id}","created_at":"2013-10-03T15:43:57Z","updated_at":"2015-03-30T19:09:02Z","pushed_at":"2015-12-15T17:52:05Z","git_url":"git://github.com/zeroedin-bill/dbal.git","ssh_url":"git@github.com:zeroedin-bill/dbal.git","clone_url":"https://github.com/zeroedin-bill/dbal.git","svn_url":"https://github.com/zeroedin-bill/dbal","homepage":"http://www.doctrine-project.org/projects/dbal","size":20875,"stargazers_count":0,"watchers_count":0,"language":"PHP","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"doctrine:master","ref":"master","sha":"069280e5e332cdb29fd7bf65244d8f87eff8ff55","user":{"login":"doctrine","id":209254,"avatar_url":"https://avatars.githubusercontent.com/u/209254?v=3","gravatar_id":"","url":"https://api.github.com/users/doctrine","html_url":"https://github.com/doctrine","followers_url":"https://api.github.com/users/doctrine/followers","following_url":"https://api.github.com/users/doctrine/following{/other_user}","gists_url":"https://api.github.com/users/doctrine/gists{/gist_id}","starred_url":"https://api.github.com/users/doctrine/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/doctrine/subscriptions","organizations_url":"https://api.github.com/users/doctrine/orgs","repos_url":"https://api.github.com/users/doctrine/repos","events_url":"https://api.github.com/users/doctrine/events{/privacy}","received_events_url":"https://api.github.com/users/doctrine/received_events","type":"Organization","site_admin":false},"repo":{"id":677464,"name":"dbal","full_name":"doctrine/dbal","owner":{"login":"doctrine","id":209254,"avatar_url":"https://avatars.githubusercontent.com/u/209254?v=3","gravatar_id":"","url":"https://api.github.com/users/doctrine","html_url":"https://github.com/doctrine","followers_url":"https://api.github.com/users/doctrine/followers","following_url":"https://api.github.com/users/doctrine/following{/other_user}","gists_url":"https://api.github.com/users/doctrine/gists{/gist_id}","starred_url":"https://api.github.com/users/doctrine/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/doctrine/subscriptions","organizations_url":"https://api.github.com/users/doctrine/orgs","repos_url":"https://api.github.com/users/doctrine/repos","events_url":"https://api.github.com/users/doctrine/events{/privacy}","received_events_url":"https://api.github.com/users/doctrine/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/doctrine/dbal","description":"Doctrine Database Abstraction Layer","fork":false,"url":"https://api.github.com/repos/doctrine/dbal","forks_url":"https://api.github.com/repos/doctrine/dbal/forks","keys_url":"https://api.github.com/repos/doctrine/dbal/keys{/key_id}","collaborators_url":"https://api.github.com/repos/doctrine/dbal/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/doctrine/dbal/teams","hooks_url":"https://api.github.com/repos/doctrine/dbal/hooks","issue_events_url":"https://api.github.com/repos/doctrine/dbal/issues/events{/number}","events_url":"https://api.github.com/repos/doctrine/dbal/events","assignees_url":"https://api.github.com/repos/doctrine/dbal/assignees{/user}","branches_url":"https://api.github.com/repos/doctrine/dbal/branches{/branch}","tags_url":"https://api.github.com/repos/doctrine/dbal/tags","blobs_url":"https://api.github.com/repos/doctrine/dbal/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/doctrine/dbal/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/doctrine/dbal/git/refs{/sha}","trees_url":"https://api.github.com/repos/doctrine/dbal/git/trees{/sha}","statuses_url":"https://api.github.com/repos/doctrine/dbal/statuses/{sha}","languages_url":"https://api.github.com/repos/doctrine/dbal/languages","stargazers_url":"https://api.github.com/repos/doctrine/dbal/stargazers","contributors_url":"https://api.github.com/repos/doctrine/dbal/contributors","subscribers_url":"https://api.github.com/repos/doctrine/dbal/subscribers","subscription_url":"https://api.github.com/repos/doctrine/dbal/subscription","commits_url":"https://api.github.com/repos/doctrine/dbal/commits{/sha}","git_commits_url":"https://api.github.com/repos/doctrine/dbal/git/commits{/sha}","comments_url":"https://api.github.com/repos/doctrine/dbal/comments{/number}","issue_comment_url":"https://api.github.com/repos/doctrine/dbal/issues/comments{/number}","contents_url":"https://api.github.com/repos/doctrine/dbal/contents/{+path}","compare_url":"https://api.github.com/repos/doctrine/dbal/compare/{base}...{head}","merges_url":"https://api.github.com/repos/doctrine/dbal/merges","archive_url":"https://api.github.com/repos/doctrine/dbal/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/doctrine/dbal/downloads","issues_url":"https://api.github.com/repos/doctrine/dbal/issues{/number}","pulls_url":"https://api.github.com/repos/doctrine/dbal/pulls{/number}","milestones_url":"https://api.github.com/repos/doctrine/dbal/milestones{/number}","notifications_url":"https://api.github.com/repos/doctrine/dbal/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/doctrine/dbal/labels{/name}","releases_url":"https://api.github.com/repos/doctrine/dbal/releases{/id}","created_at":"2010-05-20T17:01:42Z","updated_at":"2015-12-15T12:37:05Z","pushed_at":"2015-12-15T17:52:06Z","git_url":"git://github.com/doctrine/dbal.git","ssh_url":"git@github.com:doctrine/dbal.git","clone_url":"https://github.com/doctrine/dbal.git","svn_url":"https://github.com/doctrine/dbal","homepage":"http://www.doctrine-project.org/projects/dbal","size":23435,"stargazers_count":684,"watchers_count":684,"language":"PHP","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":584,"mirror_url":null,"open_issues_count":309,"forks":584,"open_issues":309,"watchers":684,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/doctrine/dbal/pulls/818"},"html":{"href":"https://github.com/doctrine/dbal/pull/818"},"issue":{"href":"https://api.github.com/repos/doctrine/dbal/issues/818"},"comments":{"href":"https://api.github.com/repos/doctrine/dbal/issues/818/comments"},"review_comments":{"href":"https://api.github.com/repos/doctrine/dbal/pulls/818/comments"},"review_comment":{"href":"https://api.github.com/repos/doctrine/dbal/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/doctrine/dbal/pulls/818/commits"},"statuses":{"href":"https://api.github.com/repos/doctrine/dbal/statuses/a22c1bf4a75cd27ec954d9e6675ab4bd5b5d8a42"}}}}