FileStorage: Fix bug retrieving the first record #672
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
UlrichEckhardt
force-pushed
the
fix/skip-first-record
branch
from
78e9826 to
6c030b8
Compare
|
Hey, thanks for the PR! I want to think a bit about how to simplify this. Btw. I might be interested in upstreaming some of the tests at some point. |
|
If you want, I can create a PR with an empty PHPUnit integration, if you'd like that detached from this one. Same goes for #671. |
UlrichEckhardt
force-pushed
the
fix/skip-first-record
branch
2 times, most recently
from
1392131 to
f4e0305
Compare
UlrichEckhardt
force-pushed
the
fix/skip-first-record
branch
from
f4e0305 to
ff4936a
Compare
UlrichEckhardt
force-pushed
the
fix/skip-first-record
branch
from
ff4936a to
7eccb7e
Compare
The file pointer is positioned at the line of the previously read record. When reading the next record, the current one is skipped. After opening the index file, the position is zero and reading the next record then skips the first record. The fix adds a flag that handles this corner-case.
UlrichEckhardt
force-pushed
the
fix/skip-first-record
branch
from
7eccb7e to
698dc71
Compare
|
I ended up simplifying the index reading logic, which also fixes this issue (c6a7b1f). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The file pointer is positioned at the line of the previously retrieved record. When retrieving the next record, the current one is skipped to get to the next record. After opening the index file, the position is zero and retrieving the next record then skips over the first record. The fix adds a simple flag that handles this corner-case.
BTW, I was wondering why the code there was implemented in this particular way. I would have positioned the file pointer at the beginning of the next record, so that it can be fetched conveniently. Is it to recover from EOF after more data is added?
BTW2: I found this trying to write a few tests, see https://github.com/UlrichEckhardt/clockwork/tree/feat/phpunit-integration. The commit from this PR is also contained in that branch.