multi_iterator looks broken

[rsr-at-mindtwin]

func (m *multiIterator) Seek(key []byte) bool {
	m.current = 0
	iters := []Iterator{}
	ok := false
	for i := range m.iters {
		if m.iters[i].Seek(key) {
			iters = append(iters, m.iters[i])
			ok = true
		}
	}
	return ok
}

In the code above, the assumption of Next() on the first call is that the set of iterators in m.iters actually are the iterators that had a hit on Seek(), hence the accumulation of iterators into the local []iterator{}.

If that's not the goal, then this code is simply wrong and the iters local shouldn't exist.

[frairon]

Hi Rob,
thanks for the issue report. That code looks indeed pretty strange, if not wrong. I'll take a look later.
If you want to improve it yourself, you're very welcome to create a PR :)

Cheers!

[frairon]

@rsr-at-mindtwin, I had a look at the iterator implementation. I think the idea was that seek indeed filters the iterators so that only the ones matching the seek will remain in the set of iterators for the next Next()-call. Then however, the original iterators would be lost for the following seek calls.
I created a PR with the changes that I think would be necessary to fix that. Take look and let me know what you think.

[rsr-at-mindtwin]

thanks. that looks right to me.

[SamiHiltunen]

There is a new multi iterator implementation here: https://github.com/lovoo/goka/blob/retention/storage/merge_iterator.go

The new implementation should preserve order when iterating over multiple sorted iterators and work as expected when seeking. While the changes in the branch are not really ready for merging otherwise, the iterator itself is complete as far as I remember. I'll have a look on the weekend and new multi iterator merged.

[SamiHiltunen]

Didn't quite have the time I expected earlier, but better late than never I guess. #185 should fix this issue.

[db7]

@frairon can you check the PR? It lgtm.