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

RedisIterator skips first key when it.Next() is called #394

Closed
chrisfjones opened this issue Sep 16, 2022 · 5 comments
Closed

RedisIterator skips first key when it.Next() is called #394

chrisfjones opened this issue Sep 16, 2022 · 5 comments

Comments

@chrisfjones
Copy link
Contributor

chrisfjones commented Sep 16, 2022

The Processor.VisitValues function expects it.Next() to be called before accessing any keys. The redisIterator doesn't behave this way and will instead skip over the first key.

@frairon
Copy link
Contributor

frairon commented Sep 16, 2022

Oh damn, that's embarassing. That should've been covered by a test. Thanks for the hint, will take care of that as soon as possible. Unless you want to give it a shot 🙄

@chrisfjones
Copy link
Contributor Author

No worries! I'm working on a PR now, just adding tests 👍

@frairon
Copy link
Contributor

frairon commented Sep 16, 2022

Awesome, thanks a lot!

chrisfjones added a commit to chrisfjones/goka that referenced this issue Sep 16, 2022
@chrisfjones
Copy link
Contributor Author

Upon further inspection, it appears the contract for storage.Iterator is that it.Next() must be called first before accessing. This is how all the other iterators appear to work, so I've just updated redisIterator to behave that way in this PR #392 👍

@chrisfjones chrisfjones changed the title VisitValues does not process the first element from the table iterator RedisIterator skips first key when it.Next() is called Sep 16, 2022
@frairon
Copy link
Contributor

frairon commented Sep 17, 2022

Now that you mention it, there was a similar discussion in PR 320 when fixing differences between different storages using Seek. So again, good catch and thanks for the fix :)

I guess we should improve the iterator here and there:

  • better documentation + examples
  • define behavior when calling Value or Key without having called Next first
  • make sure all supported storages are maintained and tested

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

No branches or pull requests

2 participants