Skip Paginator LIMIT subquery and WHERE IN if query do not have LIMIT #6306

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

Originally created by @Seb33300 on GitHub (Sep 24, 2019).

Originally assigned to: @Seb33300 on GitHub.

I ran into an issue with the Paginator which I think could be handled to prevent it.

I am using the Paginator to 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, the LIMIT clause will NOT be added to the query.
Even if the Paginator is not required in this scenario, the same part of code handle this case so we also give the query to the Paginator.

This is working property unless the query return too many results.
Because the Paginator will use a WHERE IN query 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.

Originally created by @Seb33300 on GitHub (Sep 24, 2019). Originally assigned to: @Seb33300 on GitHub. I ran into an issue with the `Paginator` which I think could be handled to prevent it. I am using the `Paginator` to 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`, the `LIMIT` clause will NOT be added to the query. Even if the `Paginator` is not required in this scenario, the same part of code handle this case so we also give the query to the `Paginator`. This is working property unless the query return too many results. Because the `Paginator` will use a `WHERE IN` query 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.
admin added the Improvement label 2026-01-22 15:30:33 +01:00
admin closed this issue 2026-01-22 15:30:33 +01:00
Author
Owner

@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?).

@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?).
Author
Owner

@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.

@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.
Author
Owner

@Seb33300 commented on GitHub (Sep 24, 2019):

I know this is useless to use the Paginator feature 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.

@Seb33300 commented on GitHub (Sep 24, 2019): I know this is useless to use the `Paginator` feature 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.
Author
Owner

@Ocramius commented on GitHub (Sep 24, 2019):

Basically you want to say that if the setMaxResults() is null, 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?

@Ocramius commented on GitHub (Sep 24, 2019): Basically you want to say that if the `setMaxResults()` is `null`, 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?
Author
Owner

@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?

@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?
Author
Owner

@lcobucci commented on GitHub (Sep 24, 2019):

@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.

@Seb33300 setting setMaxResults() to null is kind of a flag 😄.

Not sure about the example you want.

An example of use case or a pull request to implement it?

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): > @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. @Seb33300 setting `setMaxResults()` to `null` is kind of a flag :smile:. > Not sure about the example you want. > > An example of use case or a pull request to implement it? A PR with (at least) a **functional test** showing what you expect from the library. You can use this test as example: https://github.com/doctrine/orm/blob/b52ef5a1002f99ab506a5a2d6dba5a2c236c5f43/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7820Test.php
Author
Owner

@lcobucci commented on GitHub (Sep 24, 2019):

Please send it to v2.7, btw 👍

@lcobucci commented on GitHub (Sep 24, 2019): Please send it to v2.7, btw :+1:
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6306