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

Rebuild reverse index before count reverse. #3688

Merged
merged 1 commit into from
Jul 19, 2019
Merged

Conversation

martinmr
Copy link
Contributor

@martinmr martinmr commented Jul 18, 2019

Not sure if this was causing issues before, but rebuilding the count
reverse index depends on the reverse index, so I am chaning the logic to
rebuild the reverse index first, followed by the count index.

Also adding a comment to when the count reverse index is rebuilt.


This change is Reviewable

Not sure if this was causing issues before, but rebuilding the count
reverse index depends on the reverse index, so I am chaning the logic to
rebuild the reverse index first, followed by the count index.

Also adding a comment to when the count reverse index is rebuilt.
@campoy
Copy link
Contributor

campoy commented Jul 19, 2019

Is there any chance you could have a test showing how this was previously broken and now behaves correctly?

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor Author

@martinmr martinmr left a comment

Choose a reason for hiding this comment

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

@campoy Manish said it was a safe change so I'll submit it. I am going to keep working on indices so I'll look for more tests to make this feature more robust over the next few days.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@martinmr martinmr merged commit c3852a1 into master Jul 19, 2019
@martinmr martinmr deleted the martinmr/count-reverse branch July 19, 2019 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants