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

Feature request: pagination support #53

Closed
evenlee opened this issue Jun 17, 2021 · 16 comments
Closed

Feature request: pagination support #53

evenlee opened this issue Jun 17, 2021 · 16 comments
Labels
enhancement New feature or request help wanted Extra attention is needed up-for-grabs 🙏🏽 Happy to consider a pull review to address this issue
Milestone

Comments

@evenlee
Copy link

evenlee commented Jun 17, 2021

When we get items from repo, if the count of items is more than several thousands, a better way is to get data by page, can we support it?

@IEvangelist IEvangelist added enhancement New feature or request help wanted Extra attention is needed up-for-grabs 🙏🏽 Happy to consider a pull review to address this issue labels Jun 17, 2021
@IEvangelist
Copy link
Owner

Hi @evenlee - that is a great idea, I'd love to review a PR for this addition. Would you be willing to create a pull request to add this functionality?

@anujrhicstech
Copy link

Hi IEvanelist,

I was trying to create a custom pagination functionality but facing following issue, could you please help me with that
"DocType" is my partition key field

  1. for PaginationDto, First I am trying to get total count of records

if (Paginate.TotalCount == 0)
{
QueryDefinition queryCount = new QueryDefinition("SELECT count(c.DocType ) FROM c WHERE c.DocType ='" + _partitionKey + "' and c.active=true");
var count = (await _repository.GetByQueryAsync(queryCount)).Single();
result.TotalCount = count;
}
But here as the repository of account class it returns IEnumerable but according to query it should return on single integer,
How can i get only the interger (how to fetch the single value)

  1. IMPORTANT - How can i fetch/Retrive the "Continuation token" and use in the following query

         QueryDefinition queryDefinition =
             new QueryDefinition("SELECT * FROM c WHERE c.DocType ='" + _partitionKey + "' ORDER BY c.createdOn DESC OFFSET " + ((result.CurrentPage*result.PageSize)-result.PageSize) + " LIMIT " + paginate.PageSize + "");
    
         var temp = await _repository.GetByQueryAsync(queryDefinition);
         result.Items = temp;
         return result;
    

Reference :- https://medium.com/swlh/pagination-and-searching-in-asp-net-core-api-using-cosmos-db-869384a59f5
Reference 2 :https://medium.com/@gary.strange/understanding-cosmosdb-continuation-tokens-hasmoreresults-and-connectionpolicy-requesttimeouts-3ed1fadfa81d

@evenlee
Copy link
Author

evenlee commented Jun 22, 2021

Thanks @anujrhicstech for the starting. @IEvangelist I have not got time to investigate yet, will start it soon.

@deepbass
Copy link

second this, love this library and would love this feature. I'll try and carve out some time to help as it has the up for grabs tag - is there a contributor guide or something?

@IEvangelist
Copy link
Owner

I @deepbass - I haven't put together a contributor guide yet. But contributions are most successful when starting as an issue, formalizing a plan through discussion and then creating a pull request. The pull request should include unit tests, and follow all of the existing naming conventions and styling. The pull request shouldn't include changes to existing infrastructure unless explicitly stated. It sounds like this is to add a new API - wherever pagination is possible we'd add the supporting APIs.

See: https://docs.microsoft.com/azure/cosmos-db/sql-query-pagination

@emrekara37
Copy link
Contributor

Thanks @anujrhicstech for the starting. @IEvangelist I have not got time to investigate yet, will start it soon.

Hi @evenlee ,

i want to implement this feature too , if you didn't start we can start together or i can support to you

@zhangzunke
Copy link

Hi @emrekara37 and @evenlee,
I am working on this feature, the PR has been created, and ready for approval.

@anujrhicstech
Copy link

@IEvangelist , its seems @zhangzunke addressed the requested changes, are there anymore changes required ? or can you please review, merge and release it

@IEvangelist
Copy link
Owner

@IEvangelist , its seems @zhangzunke addressed the requested changes, are there anymore changes required ? or can you please review, merge and release it

Hi @anujrhicstech - I'm still reviewing it, there are a few conceptual / design decisions that I'm thinking about. But it is much closer! Stay tuned...

@anujrhicstech
Copy link

Hi @IEvangelist ,
Have you completed the review of this pull request, are any changes required.
Just awaiting get this update into my test project. Any idea when this pull request might be processed.

Thanks

@IEvangelist , its seems @zhangzunke addressed the requested changes, are there anymore changes required ? or can you please review, merge and release it

Hi @anujrhicstech - I'm still reviewing it, there are a few conceptual / design decisions that I'm thinking about. But it is much closer! Stay tuned...

Hi @IEvangelist ,
Have you completed the review of this pull request, are any changes required.
Just awaiting get this update into my test project. Any idea when this pull request might be processed.

Thanks

@IEvangelist
Copy link
Owner

Hi @anujrhicstech - we've been waffling over this for a while now and decided that the proposed PR wasn't the correct implementation. We all agree that the feature should exist, but we just need to implement it correctly. See some of the comments in the PR, and this in particular : #57 (comment)

@mumby0168
Copy link
Collaborator

Also may be good to refer to based on #57 (comment) a jump straight to the issue in the ef-core implementation for cosmos dotnet/efcore#24513

@mumby0168
Copy link
Collaborator

mumby0168 commented Jul 25, 2021

Some more information on continuation tokens here. Suppose we need to decide whether or not we take care of caching the continuation token or let this be handled by the caller of the paging implementation. I think it may be simpler.

@mumby0168
Copy link
Collaborator

@IEvangelist made this gist for a sample of paging using continuation tokens where the request charge stays the same foreach query and it knows where it left off from the continuation token. gist-here.

So as I see it we would either have to cache this value and return a Guid to the user but then how long do we keep it for? If we don't do that do we return the token to the client directly?

I am also curious that say if you are not just loading more data continuing where you left off and say wanted to skip to page 5 where the page size is 50, would we then have to use the Skip(pageSize - 1 * pageSize).Take(pageSize) don't see how the continuation tokens could help here? Would be interesting to know how EF core handle these cases and if they use maybe a IMemoryCache to cache continuation tokens.

@Rabosa616
Copy link
Contributor

I´ve modified localy your code on the pagination branch to use skip and take
I´ve removed the continuationToken and rename the lastPage by pageNumber.
Then add on the DefaultRepository.cs at line 300 this:

query = query.Skip(pageSize * (pageNumber - 1)).Take(pageSize);

I´ve did several tests and works as expected.

@mumby0168
Copy link
Collaborator

Hey, @Rabosa616 @evenlee @anujrhicstech @zhangzunke @deepbass we have just merged and released our first take on paging make use of continuation tokens see PR #142 this includes an example and works for a load more paging scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed up-for-grabs 🙏🏽 Happy to consider a pull review to address this issue
Projects
None yet
8 participants