mirror of
https://github.com/doctrine/orm.git
synced 2026-03-23 22:42:18 +01:00
Skip Paginator LIMIT subquery and WHERE IN if query do not have LIMIT #6306
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 @Seb33300 on GitHub (Sep 24, 2019).
Originally assigned to: @Seb33300 on GitHub.
I ran into an issue with the
Paginatorwhich I think could be handled to prevent it.I am using the
Paginatorto paginate a table.The user can choose the number of items by page (10 ,20, ... but also All items).
In the case the user choose to display
All items, theLIMITclause will NOT be added to the query.Even if the
Paginatoris not required in this scenario, the same part of code handle this case so we also give the query to thePaginator.This is working property unless the query return too many results.
Because the
Paginatorwill use aWHERE INquery and it could reach the limit of allowed number of parameters by the database engine.See: https://www.doctrine-project.org/projects/doctrine-orm/en/2.6/tutorials/pagination.html
This is what we are encountering on tables with thousands of items on SQL Server.
@lcobucci commented on GitHub (Sep 24, 2019):
@Seb33300 I'd say that handle this case isn't an ORM's responsibility. The paginator component is quite generic and having a flag to enable its pagination behaviour or not smells like a bad design for the library.
I'd suggest you to encapsulate this in your application (perhaps decorating the paginator?).
@TomHAnderson commented on GitHub (Sep 24, 2019):
It sounds to me like you already know if you don't need a limit clause. So, if you're not using a limit don't use a Paginator.
@Seb33300 commented on GitHub (Sep 24, 2019):
I know this is useless to use the
Paginatorfeature if we are not intended to paginate results.But I felt it more like an optimization, since this could prevent to execute useless queries.
Btw even if we do not use it to paginate results, it could also be used to count distinct entries.
@lcobucci not sure what you mean by a flag, but it just require to check if a limit has been added to the query. No extra parameter or something else required.
@Ocramius commented on GitHub (Sep 24, 2019):
Basically you want to say that if the
setMaxResults()isnull, you just want to execute the query, right?I think that may be an acceptable feature, if it doesn't raise complexity too much. Could you hack up an example, just to post it for the purposes of discussion?
@Seb33300 commented on GitHub (Sep 24, 2019):
Yes, this is exactly what I mean.
No need for filtering the original query with ids and no need for an extra query to get distinct ids since we want all of them.
Not sure about the example you want.
An example of use case or a pull request to implement it?
@lcobucci commented on GitHub (Sep 24, 2019):
@Seb33300 setting
setMaxResults()tonullis kind of a flag 😄.A PR with (at least) a functional test showing what you expect from the library.
You can use this test as example:
b52ef5a100/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7820Test.php@lcobucci commented on GitHub (Sep 24, 2019):
Please send it to v2.7, btw 👍