-
Notifications
You must be signed in to change notification settings - Fork 16
simplify pagination #472
base: master
Are you sure you want to change the base?
simplify pagination #472
Conversation
6fb3783
to
1b8e821
Compare
eb1ba8e
to
08c436b
Compare
That looks good! |
Sounds good for Typescript – I guess for the others like Go, it can be a runtime error. |
Might I add that what we usually need in pagination is also the total number of records. Would be interesting to see if some optimization could be done about that. |
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.
The spec is not really a spec, it leaves way too much room for interpretation and speculation:
- Do I need to provide both
cursor
andtake
? If no, which combination is valid? What are the implications? - In which context is pagination supposed to be usable? findOne, findMany, delete, ... ?
- What happens if the cursor doesn't exist? Do different operations have different behavior?
- How is pagination influenced by other query arguments? When is it applied? Ordering plays a major role here.
Thanks for the feedback! I'll answer these questions here then persist it in the PR when everything is resolved. Do I need to provide both cursor and take? If no, which combination is valid? What are the implications?
In which context is pagination supposed to be usable? findOne, findMany, delete, ... ? All the contexts where we currently have:
This PR replaces these. What happens if the cursor doesn't exist? Do different operations have different behavior? In general, same order/behavior as the previous implementation. We're simplifying the concept, not changing the concept. If this doesn't answer the questions above for you, maybe we can talk about specific examples/issues? |
A broader point: While I knew most of the answers beforehand, the spec doesn't know any of these. I couldn't hand off the implementation to somebody without knowledge of the current QE behavior, which makes the spec in itself insufficient in my opinion. Moving on to an actual question: I find it surprising that |
Ah yes good catch. You're absolutely right! e.g. Take last 10 users prisma.users.findMany({
take: -10
}) Take first 10 users prisma.users.findMany({
take: 10
}) |
What happens to the case of having both the |
@Sytten that doesn't currently work does it? It's definitely an interesting use case, but I think it's pretty obscure and I'm don't think we'd be able to write efficient queries for it. |
I don't know, but I expect it would not work. thinking about the query, I think it can only work with numerical IDs since they are naturally ordered. So no, not a usecase for prisma IMO. |
Additional comment, motivated by @mavilein: Skip is also not really mentioned here. It's required for any kind of count-based pagination to actually work.
Is this a correct assumption that this should still be possible @matthewmueller? |
Good call, yep. I think it would look like this. Please make sure it matches your expectations. Also thought about negative skips, but I think that could get confusing. I'll update the spec after.
|
Not really what I expected. Just setting a cursor should set the starting point of the read in the list. Everything after and including |
Good catch. Completely agree. Without take, there's no limit. |
Implemented in: prisma/prisma-engines#767 |
Shouldn't this be merged now? |
Not yet. I still need to write down some of the on-the-fly decisions we made during implementation. |
This PR attempts to further simplify @sorenbs's proposal by supporting a negative
take
value. This is similar to how JS also supportsarr.slice(-5)
Rendered View
Open Questions:
at: 5
instead ofcursor: 5
?These are questions that are relevant to previous implementations and proposals, but I think it's important to list them out here.
take: 0
should mean take none or take at the cursor? and iftake: 0
takes 1, thentake: 3
would actually return 4 items?{ cursor: 5, take: undefined }
, is thistake: 0
ortake: 1
or not allowed?