-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add an option to limit results with Elastica #213
Conversation
Thank for this, @odolbeau. I'm sorry there's been no activity from us here. Please could you add something to the Elastica section in the README explaining this new option? This will then be good to merge, I think. |
28c49da
to
b4a5448
Compare
PR rebased! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this. Just one last change.
README.md
Outdated
@@ -294,6 +294,10 @@ $query = new Query::create(new Term(array( | |||
$adapter = new ElasticaAdapter($searchable, $query); | |||
``` | |||
|
|||
*Be careful when paginating a huge set of documents. By default, offset + limit | |||
can't exceed 10000. For more information, see: | |||
[#213](https://github.com/whiteoctober/Pagerfanta/pull/213#issue-87631892).* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add an additional sentence saying something like "You can mitigate this by setting the $maxResults
parameter when constructing the ElasticaAdapter
"? Thanks
b4a5448
to
cb95185
Compare
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thanks.
I'll merge once tests have passed. |
I've created release https://github.com/whiteoctober/Pagerfanta/releases/tag/v2.0.1 for this. Thanks again for your input. |
With recent ES version, we have en exception when size + offset > 10000.
BTW it's possible to override this directly in ES configuration even if it's not recommended.
With this new constructor argument, it's possible to limit the number of results even if the dataset contains more results.
In this case, we won't display pages where size + offset > 10000 and we won't have this error: