Skip to content

Fix pagination + multipart indexes #177

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

Merged
merged 15 commits into from
Jul 2, 2021
Merged

Fix pagination + multipart indexes #177

merged 15 commits into from
Jul 2, 2021

Conversation

mRrvz
Copy link
Contributor

@mRrvz mRrvz commented Jun 29, 2021

Fixed invalid results for pagination queries when filtering on
a parts of a composite index. Now, instead of "scrolling"
to the desired tuple (during pagination) using the
scroll_to_after_tuple function, we change the iterators
(EQ -> GE, REQ -> LT) to use them in
index:pairs function, together with a combination with
additional filtration. Closes #170

@mRrvz mRrvz marked this pull request as draft June 29, 2021 23:41
@mRrvz mRrvz marked this pull request as ready for review June 30, 2021 10:27
@mRrvz mRrvz requested review from olegrok and ligurio June 30, 2021 10:30
Copy link
Member

@ligurio ligurio left a comment

Choose a reason for hiding this comment

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

@mRrvz Alexey, thanks for the patch!

Few comments:

  • could you add explanations to commit messages "Fix unnecessary sharding key", "Remove unnecessary test". For me it is not clear what was fixed regarding to sharding key and why we need to remove test.
  • I suppose the item to changelog should be added in the same commit where fix was added.

@mRrvz mRrvz requested a review from olegrok June 30, 2021 14:46
@mRrvz mRrvz requested a review from olegrok June 30, 2021 21:41
@mRrvz mRrvz requested a review from olegrok June 30, 2021 22:05
Copy link
Contributor

@olegrok olegrok left a comment

Choose a reason for hiding this comment

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

Thanks for your patch. Please resolve the rest of not my comments.

Copy link
Contributor

@dokshina dokshina left a comment

Choose a reason for hiding this comment

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

Please make Changelog more informative.

@dokshina dokshina requested a review from ligurio July 1, 2021 15:23
@ligurio ligurio force-pushed the reverse-pagination-fix branch from 30aa400 to ab46af8 Compare July 2, 2021 14:57
@mRrvz mRrvz force-pushed the reverse-pagination-fix branch from ab46af8 to 4d0f002 Compare July 2, 2021 15:25
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.

[8pt] Pagination [+ multipart indexes] is broken
4 participants