chore: revise goleveldb iterator (tendermint#12)

goleveldb/db.go

@@ -5,7 +5,6 @@ import (
 	"path/filepath"
 
 	tmdb "github.com/line/tm-db/v2"
-	tmutil "github.com/line/tm-db/v2/internal/util"
 	"github.com/syndtr/goleveldb/leveldb"
 	"github.com/syndtr/goleveldb/leveldb/errors"
 	"github.com/syndtr/goleveldb/leveldb/opt"
@@ -172,12 +171,15 @@ func (db *GoLevelDB) Iterator(start, end []byte) (tmdb.Iterator, error) {
 		return nil, tmdb.ErrKeyEmpty
 	}
 	itr := db.db.NewIterator(&util.Range{Start: start, Limit: end}, nil)
-	return newGoLevelDBIterator(itr, start, end, false), nil
+	return newGoLevelDBIterator(itr, false), nil
 }
 
 func (db *GoLevelDB) PrefixIterator(prefix []byte) (tmdb.Iterator, error) {
-	start, end := tmutil.PrefixRange(prefix)
-	return db.Iterator(start, end)
+	if prefix != nil && len(prefix) == 0 {
+		return nil, tmdb.ErrKeyEmpty
+	}
+	itr := db.db.NewIterator(util.BytesPrefix(prefix), nil)
+	return newGoLevelDBIterator(itr, false), nil
 }
 
 // ReverseIterator implements DB.
@@ -186,10 +188,13 @@ func (db *GoLevelDB) ReverseIterator(start, end []byte) (tmdb.Iterator, error) {
 		return nil, tmdb.ErrKeyEmpty
 	}
 	itr := db.db.NewIterator(&util.Range{Start: start, Limit: end}, nil)
-	return newGoLevelDBIterator(itr, start, end, true), nil
+	return newGoLevelDBIterator(itr, true), nil
 }
 
 func (db *GoLevelDB) ReversePrefixIterator(prefix []byte) (tmdb.Iterator, error) {
-	start, end := tmutil.PrefixRange(prefix)
-	return db.ReverseIterator(start, end)
+	if prefix != nil && len(prefix) == 0 {
+		return nil, tmdb.ErrKeyEmpty
+	}
+	itr := db.db.NewIterator(util.BytesPrefix(prefix), nil)
+	return newGoLevelDBIterator(itr, true), nil
 }

goleveldb/db_test.go

@@ -38,7 +38,7 @@ func TestGoLevelDBNewDBAndNewReadonlyDB(t *testing.T) {
 }
 
 func TestGoLevelDBStats(t *testing.T) {
-	name, dir := dbtest.NewTestName("cleveldb")
+	name, dir := dbtest.NewTestName("goleveldb")
 	db, err := NewDB(name, dir)
 	defer dbtest.CleanupDB(db, name, dir)
 	require.NoError(t, err)
@@ -64,6 +64,15 @@ func TestGoLevelDBEmptyIterator(t *testing.T) {
 	dbtest.TestDBEmptyIterator(t, db)
 }
 
+func TestGoLevelDBPrefixIterator(t *testing.T) {
+	name, dir := dbtest.NewTestName("goleveldb")
+	db, err := NewDB(name, dir)
+	defer dbtest.CleanupDB(db, name, dir)
+	require.NoError(t, err)
+
+	dbtest.TestDBPrefixIterator(t, db)
+}
+
 func TestGoLevelDBPrefixIteratorNoMatchNil(t *testing.T) {
 	name, dir := dbtest.NewTestName("goleveldb")
 	db, err := NewDB(name, dir)

goleveldb/iterator.go

@@ -1,93 +1,34 @@
 package goleveldb
 
 import (
-	"bytes"
-
 	tmdb "github.com/line/tm-db/v2"
 	"github.com/line/tm-db/v2/internal/util"
 	"github.com/syndtr/goleveldb/leveldb/iterator"
 )
 
 type goLevelDBIterator struct {
 	source    iterator.Iterator
-	start     []byte
-	end       []byte
 	isReverse bool
-	isInvalid bool
 }
 
 var _ tmdb.Iterator = (*goLevelDBIterator)(nil)
 
-func newGoLevelDBIterator(source iterator.Iterator, start, end []byte, isReverse bool) *goLevelDBIterator {
-	if isReverse {
-		if end == nil {
-			source.Last()
-		} else {
-			valid := source.Seek(end)
-			if valid {
-				eoakey := source.Key() // end or after key
-				if bytes.Compare(end, eoakey) <= 0 {
-					source.Prev()
-				}
-			} else {
-				source.Last()
-			}
-		}
+func newGoLevelDBIterator(source iterator.Iterator, isReverse bool) *goLevelDBIterator {
+	if !isReverse {
+		source.First()
 	} else {
-		if start == nil {
-			source.First()
-		} else {
-			source.Seek(start)
-		}
+		source.Last()
 	}
+
 	return &goLevelDBIterator{
 		source:    source,
-		start:     start,
-		end:       end,
 		isReverse: isReverse,
-		isInvalid: false,
 	}
 }
 
 // Valid implements Iterator.
 func (itr *goLevelDBIterator) Valid() bool {
-
-	// Once invalid, forever invalid.
-	if itr.isInvalid {
-		return false
-	}
-
-	// If source errors, invalid.
-	if err := itr.Error(); err != nil {
-		itr.isInvalid = true
-		return false
-	}
-
-	// If source is invalid, invalid.
-	if !itr.source.Valid() {
-		itr.isInvalid = true
-		return false
-	}
-
-	// If key is end or past it, invalid.
-	var start = itr.start
-	var end = itr.end
-	var key = itr.source.Key()
-
-	if itr.isReverse {
-		if start != nil && bytes.Compare(key, start) < 0 {
-			itr.isInvalid = true
-			return false
-		}
-	} else {
-		if end != nil && bytes.Compare(end, key) <= 0 {
-			itr.isInvalid = true
-			return false
-		}
-	}
-
-	// Valid
-	return true
+	return itr.source.Valid()
 }
 
 // Key implements Iterator.
@@ -109,10 +50,10 @@ func (itr *goLevelDBIterator) Value() []byte {
 // Next implements Iterator.
 func (itr *goLevelDBIterator) Next() {
 	itr.assertIsValid()
-	if itr.isReverse {
-		itr.source.Prev()
-	} else {
+	if !itr.isReverse {
 		itr.source.Next()
+	} else {
+		itr.source.Prev()
 	}
 }
 
@@ -127,7 +68,7 @@ func (itr *goLevelDBIterator) Close() error {
 	return nil
 }
 
-func (itr goLevelDBIterator) assertIsValid() {
+func (itr *goLevelDBIterator) assertIsValid() {
 	if !itr.Valid() {
 		panic("iterator is invalid")
 	}

internal/dbtest/helper.go

@@ -217,6 +217,45 @@ func TestDBEmptyIterator(t *testing.T, db tmdb.DB) {
 	verifyAndCloseIterator(t, ritr, nil, "reverse iterator with empty db")
 }
 
+func TestDBPrefixIterator(t *testing.T, db tmdb.DB) {
+	for i := 0; i < 10; i++ {
+		if i != 6 { // but skip 6.
+			err := db.Set(Int642Bytes(int64(i)), []byte{})
+			require.NoError(t, err)
+		}
+	}
+
+	// Blank iterator keys should error
+	_, err := db.PrefixIterator([]byte{})
+	require.Equal(t, tmdb.ErrKeyEmpty, err)
+	_, err = db.ReversePrefixIterator([]byte{})
+	require.Equal(t, tmdb.ErrKeyEmpty, err)
+
+	itr, err := db.PrefixIterator(nil)
+	require.NoError(t, err)
+	verifyAndCloseIterator(t, itr, []int64{0, 1, 2, 3, 4, 5, 7, 8, 9}, "forward iterator")
+
+	ritr, err := db.ReversePrefixIterator(nil)
+	require.NoError(t, err)
+	verifyAndCloseIterator(t, ritr, []int64{9, 8, 7, 5, 4, 3, 2, 1, 0}, "reverse iterator")
+
+	itr, err = db.PrefixIterator(Int642Bytes(0))
+	require.NoError(t, err)
+	verifyAndCloseIterator(t, itr, []int64{0}, "forward iterator with 0 prefix")
+
+	itr, err = db.ReversePrefixIterator(Int642Bytes(0))
+	require.NoError(t, err)
+	verifyAndCloseIterator(t, itr, []int64{0}, "reverse iterator with 0 prefix")
+
+	itr, err = db.PrefixIterator(Int642Bytes(6))
+	require.NoError(t, err)
+	verifyAndCloseIterator(t, itr, nil, "forward iterator with 6 prefix")
+
+	itr, err = db.ReversePrefixIterator(Int642Bytes(6))
+	require.NoError(t, err)
+	verifyAndCloseIterator(t, itr, nil, "reverse iterator with 6 prefix")
+}
+
 func verifyAndCloseIterator(t *testing.T, itr tmdb.Iterator, expected []int64, msg string) {
 	var list []int64
 	for itr.Valid() {

makefile

@@ -56,7 +56,7 @@ bench-memdb:
 	@go test -bench=. ./memdb/... -tags memdb
 
 bench-goleveldb:
-	@go test -bench=. ./goleveldb/... -tags goleveldb -v
+	@go test -bench=. ./goleveldb/... -tags goleveldb
 
 bench-cleveldb:
 	@go test -bench=. ./cleveldb/... -tags cleveldb