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

Quick fix for the second half of the assertion bug #211

Merged
merged 1 commit into from Mar 19, 2019

Conversation

joka921
Copy link
Member

@joka921 joka921 commented Mar 18, 2019

  • same fixing strategy also for the function that gets the following block.

  • Also fixed problems in case the scanNonFunctionalRelation etc. functions get an empty input.

  • the whole bloated scanPOS, scanPSO, ... functions in the Index class clearly need refactoring, but not tonight.

- same fixing strategy also for the function that gets the following block.

- Also fixed problems in case the scanNonFunctionalRelation etc. functions get an empty input.

- the whole bloated scanPOS, scanPSO, ... functions in the Index class clearly need refactoring, but not tonight.
@niklas88
Copy link
Member

niklas88 commented Mar 19, 2019

Besides the scanXXX() methods I think the two functions we touched for this bug are really hard to grasp too. But I think this fix is minimally invasive and I've tested that this indeed fixes #206.

That said I still don't understand the logic around the it == _blocks.end() case. In that case we decrement it, then don't increment it again and then set after to _startRhs. That however means we get the exact same result as for getBlockStartAndNofBytesForLhs() (as we are skipping the increment which is the only difference). Does that make sense is the last block its own follow block?

EDIT: Looking at the docs for std::lower_bound() I was confused for a bit how we could end up with it == _blocks.end() I understand this only happens for an Id larger than any in the _blocks. Shouldn't that also mean we have an empty result?

EDIT2: Nevermind it's in the function comment for getFollowBlockForLhs()

Copy link
Member

@niklas88 niklas88 left a comment

Choose a reason for hiding this comment

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

As you said these functions as well as scanXXX() are quite confusing and duplicate code. But this is a task for another day and this fix is minimally invasive and fixes #206 so let's merge this now.

@niklas88 niklas88 merged commit cf8669c into ad-freiburg:master Mar 19, 2019
@joka921 joka921 deleted the f.AssertionBug2 branch May 8, 2021 09:15
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