mirror of
https://github.com/doctrine/orm.git
synced 2026-03-23 22:42:18 +01:00
DDC-3575: Paginator's CountOutputWalker keeps the ORDER BY in the subquery for all non-MSSQL platforms #4397
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @doctrinebot on GitHub (Feb 20, 2015).
Originally assigned to: @beberlei on GitHub.
Jira issue originally created by user jflatnes:
The Paginator's CountOutputWalker leaves the ORDER BY clause in the subquery that's executed to get the total count.
The original (and current) version of that code included the following comment:
{quote}
Note that the ORDER BY clause is not removed. Many SQL implementations (e.g. MySQL) are able to cache subqueries. By keeping the ORDER BY clause intact, the limitSubQuery that will most likely be executed next can be read from the native SQL cache.
{quote}
My understanding is that MySQL does not, in fact, cache subqueries in the query cache, so cached results are not available between executions of different queries. The current MySQL manual still states that "Queries that are a subquery of an outer query" are not cached.
This pull request was merged in 2013 to address errors with mssql and paginating ordered queries. Midway through the discussion on that issue, it was determined that it would be better to just remove the ORDER BY clause for all platforms, not just MS SQL. However, that decision was reversed when it came time to merge the PR.
Does the comment on that method accurately describe the rationale for retaining the ORDER BY clause for the count query? If so, am I simply wrong about how MySQL operates, or does the comment refer to some other platform that benefits from this pattern?
@doctrinebot commented on GitHub (Feb 20, 2015):
@doctrinebot commented on GitHub (Feb 20, 2015):
Comment created by @ocramius:
See https://github.com/doctrine/doctrine2/pull/1283
This change was reverted because MSSQL is pretty much a niche, and fixing bugs for it is really making things complicated for all the other storages.
@doctrinebot commented on GitHub (Feb 20, 2015):
Comment created by jflatnes:
Thanks for the reply, Marco.
I think this is getting conflated with the other Paginator-related ORDER BY changes and reversions that have happened breaking and fixing the actual sorting behavior. The pull you linked looks like its relating to the LimitSubqueryOutputWalker, not the CountOutputWalker.
The thing I'm talking about is still there, and does specifically address only a MSSQL bug: https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/Tools/Pagination/CountOutputWalker.php#L82
The particular part of the discussion on PR #572 I was referring to above is here: https://github.com/doctrine/doctrine2/pull/572#discussion_r3154962
My concern is that the rationale for keeping the ORDER BY intact for all the other platforms (only when running the count query, not the "real" one) is a performance-related one, and that it's a flawed rationale. (Or, at least, that the comment about it is.)