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

Cursor-based paging WIP. #14971

Conversation

benmccallum
Copy link
Contributor

@benmccallum benmccallum commented Mar 10, 2019

This is just a WIP on a solution to #14104.
Just a draft for now so we can discuss the changes I'm making informally with a nice diff.

@dnfclas
Copy link

dnfclas commented Mar 10, 2019

CLA assistant check
All CLA requirements met.

@ajcvickers
Copy link
Member

@divega to follow up.

@benmccallum
Copy link
Contributor Author

I'd love some advice on whether the approach I'm taking is actually the correct one; see the issue for my latest comment on what I've done so far. Thanks team!

@smitpatel
Copy link
Member

@benmccallum - Looking at PR, I would suggest holding off for now. I see that you have added new Queryable operator for Take(Before/After)Match and plumbed it through Relinq parser though there is no translation further. In 3.0, we are going away from Relinq QueryModel hence some of it would be unnecessary and translation would be directly from expression tree. The new pipeline without QM is being worked in #14455. In few weeks it should be ready to be merged and then we can add new features to query.

@benmccallum benmccallum mentioned this pull request Mar 12, 2019
@benmccallum
Copy link
Contributor Author

Awesome, thx for the heads up @smitpatel. Will hold off a bit longer!

@benmccallum
Copy link
Contributor Author

Closing as per above.

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