diff --git a/table/merge_iterator.go b/table/merge_iterator.go index b42d79e03..14bafced8 100644 --- a/table/merge_iterator.go +++ b/table/merge_iterator.go @@ -139,10 +139,17 @@ func (mi *MergeIterator) swapSmall() { // Next returns the next element. If it is the same as the current key, ignore it. func (mi *MergeIterator) Next() { - for mi.Valid() { - if !bytes.Equal(mi.small.key, mi.curKey) { - break - } + if !mi.Valid() { + return + } + // Invariant on entry: mi.small.key == mi.curKey byte-for-byte, since every + // public method (Rewind/Seek/Next) that leaves the iterator Valid ends by + // calling setCurrent(). Skip the always-true first comparison and advance + // unconditionally; then keep advancing while the new small.key duplicates + // the saved curKey (only possible from the OTHER child iterator). + mi.small.next() + mi.fix() + for mi.Valid() && bytes.Equal(mi.small.key, mi.curKey) { mi.small.next() mi.fix() } diff --git a/table/merge_iterator_test.go b/table/merge_iterator_test.go index c19a3dc6f..370f25345 100644 --- a/table/merge_iterator_test.go +++ b/table/merge_iterator_test.go @@ -273,6 +273,22 @@ func TestMergeIteratorSeekInvalidReversed(t *testing.T) { closeAndCheck(t, mergeIt, 4) } +// TestMergeIteratorNextOnInvalid exercises the early-return guard in Next() +// for the case where Next is called on an iterator that has already become +// invalid (Seek past the end). The call must be a safe no-op. +func TestMergeIteratorNextOnInvalid(t *testing.T) { + it1 := newSimpleIterator([]string{"1", "3"}, []string{"a1", "a3"}, false) + it2 := newSimpleIterator([]string{"2", "4"}, []string{"b2", "b4"}, false) + mergeIt := NewMergeIterator([]y.Iterator{it1, it2}, false) + // Seek past the end so the iterator is invalid. + mergeIt.Seek([]byte("z")) + require.False(t, mergeIt.Valid()) + // Next on an invalid iterator must not panic and must remain invalid. + mergeIt.Next() + require.False(t, mergeIt.Valid()) + closeAndCheck(t, mergeIt, 2) +} + func TestMergeIteratorDuplicate(t *testing.T) { it1 := newSimpleIterator([]string{"0", "1", "2"}, []string{"a0", "a1", "a2"}, false) it2 := newSimpleIterator([]string{"1", "3"}, []string{"b1", "b3"}, false)