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

Pagination support in dbcore #750

Open
PierreRust opened this issue May 6, 2014 · 5 comments
Open

Pagination support in dbcore #750

PierreRust opened this issue May 6, 2014 · 5 comments
Labels
feature features we would like to implement

Comments

@PierreRust
Copy link
Contributor

The creation of this issue has been discussed in #736.
To implement pagination properly in the HTTP API, we need to add support for pagination in dbCore. Ideally the pagination mechanism should also be usable with the command-line interface.

Ideas about pagination have been discussed in #718. It seems were are basically 4 options :

  • the naive approach : using start/offset parameters in query. While this is trivial to implement it is definitely not very efficient.
  • use a seek-able id in the query ( i.e. "gives me the next items starting with this one") : this is the approach recommended by sqlite, we have to make a small prototype to make sure it's doable with complex queries.
  • when performing a query, store temporarily the result and return a token to the client, which can be used to walk through the ResultSet. The difficult part here is to maintain, and discard at some point, the ResultSets
  • last solution : only return id for complex queries. This looks nice but I'm afraid it might not play well with the current query mechanism, which fetches full rows anyway.
@sampsyo
Copy link
Member

sampsyo commented May 6, 2014

Good plan.

Even though offset/limit is clearly the most straightforward API, I do tend to agree that the anchor-item approach is the most future-scalable. Perhaps the methods can be made to accept either a full Model instance (wherein we will look up the necessary fields for constructing the inequality in the WHERE clause) or an ID (which will cause an extra query to first look up the Model).

Of course, we will also need a (slow) alternative for slow (in-Python) queries. It won't be too hard to just skip over all rows before the anchor item is seen.

I am so not an expert in this area, though. Folks with more experience writing web apps, for example, probably have better opinions than I do.

@pjones
Copy link

pjones commented Jan 5, 2015

This would be especially useful for the Smart Playlist and Random plug-ins. Right now there's no way to limit the number of tracks generated by the Smart Playlist plug-in.

@jonathanthomas83
Copy link

Sorry guys, I'm not sure if I'm barking up the wrong tree here, and please tell me to get lost, but from a UI perspective, pagination or pager navigation isn't the most user friendly interface to have to use - we've done a lot of usability testing and found that they're not as straightforward as we'd like to think. You'd be surprised at the types of feedback, which I can share if you need me to.

A lot of the stuff mentioned here goes right over my head, but after reading #738, it seems like pagers/paged navigation methods will play a part in the proposed UI implementations. Can we reconsider their use and look at alternative options before making a decision - like I said, in terms of UI. Many thanks.

@sampsyo
Copy link
Member

sampsyo commented Apr 1, 2015

Yes! I absolutely agree that it's not a foregone conclusion that the interface should paginate. But it's still useful for the API to admit pagination for efficiency reasons—independent of the interface.

@jonathanthomas83
Copy link

That's good news, as long as we're on the same "page" ;-)

FWIW, our guys are still developing paging on the API too, makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature features we would like to implement
Projects
None yet
Development

No branches or pull requests

4 participants