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

Track real counts for limit and skip #478

Merged
merged 1 commit into from
Jan 12, 2021
Merged

Conversation

carsonfarmer
Copy link
Member

Applies Limit and Skip after other conditions and read filter have been applied. This leads to potentially slightly less efficient queries (because the child query engine can't handle the limits and skips directly), but provides more consistent and accurate/expected results. A test has been added that captures the issue presented in #458, plus a "regression" test was added to ensure we don't over-compensate.

Fixes #458.

Technically this is a bug fix, but it also changes how results are returned when applying limit and skip, so this might need to come with an appropriate version bump?

@carsonfarmer carsonfarmer added the bug Something isn't working label Jan 8, 2021
@carsonfarmer carsonfarmer self-assigned this Jan 8, 2021
Copy link
Member

@sanderpick sanderpick left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for jumping in there 💪

@carsonfarmer carsonfarmer merged commit 16c625a into master Jan 12, 2021
@carsonfarmer carsonfarmer deleted the carson/limit-skip branch January 12, 2021 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

limit and skip operation is applied before conditions
2 participants