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

fixes entry insert when index is reverted at the same time #2

Merged
merged 3 commits into from Oct 7, 2013
Merged

fixes entry insert when index is reverted at the same time #2

merged 3 commits into from Oct 7, 2013

Conversation

tdesvenain
Copy link
Contributor

Hi, this commit fixes a critical problem i encountered. The value inserted at the moment when the index is inverted was wrong (the value was set as false instead of true at the moment index value was inverted from false to true)

do you have an opinion about this ?

the problem is that i am not 100% sure that this commit is good. Obviously it won't introduce a regression (this looks more robust) but i coul'nt write a test to prove the problem i encountered can be reproduced.

@tdesvenain
Copy link
Contributor Author

FYI, i reproduce the issue into legacy code tests. those tests pass using my patch or using FieldIndex, and don't pass using not patched BooleanIndex

  when an existing content was reindexed with an opposite value.
  Fixes LP #1236354.
@tdesvenain
Copy link
Contributor Author

Thank you

Actually, it was really more complicated, problem occured when when index inversion occured
when an existing content was reindexed with a different value than before.
I added a test that tests reindexation.

insertForwardEntry was not the problem, but the fact that invert_index method uses the value of unindex length that was updated after. actually we have to update unindex before we may invert the index

@tdesvenain
Copy link
Contributor Author

Can you review and merge if possible ?

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.

None yet

2 participants