Pagination LimitSubqueryWalker optimization #5765

Closed
opened 2026-01-22 15:17:12 +01:00 by admin · 7 comments
Owner

Originally created by @plfort on GitHub (Nov 9, 2017).

Originally assigned to: @guilhermeblanco on GitHub.

LimitSubqueryWalker create useless select expressions when "resultVariable" are involved : a82f6c5725/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php (L76-L82)

We need to "Preserve mixed data in query for ordering." only if these fields are used in the "order by" clause.

Originally created by @plfort on GitHub (Nov 9, 2017). Originally assigned to: @guilhermeblanco on GitHub. LimitSubqueryWalker create useless select expressions when "resultVariable" are involved : https://github.com/doctrine/doctrine2/blob/a82f6c5725cb31348eae0b4414e8992b1400ba30/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php#L76-L82 We need to "Preserve mixed data in query for ordering." only if these fields are used in the "order by" clause.
admin added the Improvement label 2026-01-22 15:17:12 +01:00
admin closed this issue 2026-01-22 15:17:12 +01:00
Author
Owner

@plfort commented on GitHub (Nov 22, 2017):

The current behavior produces horrible performances when a lot of subselect are performed in the select clause.

@plfort commented on GitHub (Nov 22, 2017): The current behavior produces horrible performances when a lot of subselect are performed in the select clause.
Author
Owner

@mvrhov commented on GitHub (Nov 22, 2017):

I don't remember exactly but.

  • The double sort was kept because the PHP would die with core dump. At least this was the case with both 5.3 and 5.4 versions at that time.
  • The postgresql requires this and also every other modern db including the latest MySQL version.
@mvrhov commented on GitHub (Nov 22, 2017): I don't remember exactly but. * The double sort was kept because the PHP would die with core dump. At least this was the case with both 5.3 and 5.4 versions at that time. * The postgresql requires this and also every other modern db including the latest MySQL version.
Author
Owner

@plfort commented on GitHub (Nov 22, 2017):

@mvrhov Sorry but I don't get the point, can you be more specific ?

@plfort commented on GitHub (Nov 22, 2017): @mvrhov Sorry but I don't get the point, can you be more specific ?
Author
Owner

@mvrhov commented on GitHub (Nov 22, 2017):

I just wanted to tell you, that there is the reason that this was kept.

@mvrhov commented on GitHub (Nov 22, 2017): I just wanted to tell you, that there is the reason that this was kept.
Author
Owner

@plfort commented on GitHub (Nov 23, 2017):

Ok but what do you mean with "postgresql requires this ...".

@plfort commented on GitHub (Nov 23, 2017): Ok but what do you mean with "postgresql requires this ...".
Author
Owner

@mvrhov commented on GitHub (Nov 23, 2017):

That you should run the same query on whatever database you are using and the postgresql.
Most complaints here are from ppl. using MySQL which is not SQL standard conformant.

@mvrhov commented on GitHub (Nov 23, 2017): That you should run the same query on whatever database you are using and the postgresql. Most complaints here are from ppl. using MySQL which is not SQL standard conformant.
Author
Owner

@Ocramius commented on GitHub (Dec 11, 2017):

Handled in #6820

@Ocramius commented on GitHub (Dec 11, 2017): Handled in #6820
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#5765