Skip to content
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

[RFC] Lazy adapters implementation #183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Koc
Copy link

@Koc Koc commented Jun 26, 2015

Use case: some databases/search engines aren't required execute two queries for getting count and slice. So we can use only one query for slice and count, but now it doesn't work properly, because Pagerfanta filters current page so early, before slice query executed.

For preventing +1 query I've created LazyAdapterInterface that marks adapter as lazy.

TODO:

  • documentation
  • tests

@Koc
Copy link
Author

Koc commented Jun 26, 2015

Example of lazy adapter for Sphinxsearch engine https://github.com/Koc/Sphinxy/blob/master/Pagerfanta/Adapter/SphinxyQbAdapter.php

@mablae
Copy link

mablae commented Jun 26, 2015

Looks interesting for elasticsearch, too:

For example: I need aggregations from PagerfantaAdpter which are the same on all pages.

EDIT: Your PR is missing the actual LazyAdapterInterface ?


if ($this->adapter instanceof LazyAdapterInterface) {
$page = $this->filterOutOfRangeCurrentPage($this->getCurrentPage());
if ($page != $this->getCurrentPage()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should use a string comparison

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean (string)$page !== (string)$this->getCurrentPage()?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

he probably meant strict

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I meant strict

@Koc
Copy link
Author

Koc commented Jun 27, 2015

@mablae LazyAdapterInterface is present, but it is empty. It is just mark adapter as lazy.

@Koc Koc force-pushed the lazy-adapters branch from 98cbad9 to 7171cfa Compare June 29, 2015 09:40
@Koc
Copy link
Author

Koc commented Jun 29, 2015

Tests have been added

@sauron918
Copy link

Unfortunately the problem with double queries is still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants