From ef88e2b7a96a100dedabe66278f3ba809d08a516 Mon Sep 17 00:00:00 2001 From: Harshil Goel Date: Thu, 12 Sep 2024 11:39:06 +0530 Subject: [PATCH 1/4] fix: fix reverse iterator broken by seek --- db_test.go | 32 ++++++++++++++++++++++++++++++++ iterator.go | 10 +++++----- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/db_test.go b/db_test.go index ca2803874..5a535db4d 100644 --- a/db_test.go +++ b/db_test.go @@ -164,6 +164,38 @@ func runBadgerTest(t *testing.T, opts *Options, test func(t *testing.T, db *DB)) test(t, db) } +func TestReverseIterator(t *testing.T) { + runBadgerTest(t, nil, func(t *testing.T, db *DB) { + key := make([]byte, 6) + db.Update(func(txn *Txn) error { + binary.BigEndian.PutUint16(key, 5) + binary.BigEndian.PutUint32(key[2:], 1) + txn.Set(key, []byte("value1")) + + binary.BigEndian.PutUint32(key[2:], 2) + txn.Set(key, []byte("value2")) + return nil + }) + + db.View(func(txn *Txn) error { + searchBuffer := make([]byte, 3) + binary.BigEndian.PutUint16(searchBuffer, 5) + searchBuffer[2] = 0xFF + + iteratorOptions := DefaultIteratorOptions + iteratorOptions.Reverse = true + iteratorOptions.PrefetchValues = false + iteratorOptions.Prefix = searchBuffer + it := txn.NewIterator(iteratorOptions) + defer it.Close() + + it.Rewind() + require.Equal(t, it.Item().Key(), key) + return nil + }) + }) +} + func TestWrite(t *testing.T) { runBadgerTest(t, nil, func(t *testing.T, db *DB) { for i := 0; i < 100; i++ { diff --git a/iterator.go b/iterator.go index a4a611001..2232381bc 100644 --- a/iterator.go +++ b/iterator.go @@ -589,7 +589,7 @@ func (it *Iterator) Next() { // Set next item to current it.item = it.data.pop() - for it.iitr.Valid() && hasPrefix(it.iitr, it.opt.Prefix) { + for it.iitr.Valid() && (!it.opt.Reverse && hasPrefix(it)) { if it.parseItem() { // parseItem calls one extra next. // This is used to deal with the complexity of reverse iteration. @@ -725,9 +725,9 @@ func (it *Iterator) fill(item *Item) { } } -func hasPrefix(it y.Iterator, prefix []byte) bool { - if len(prefix) > 0 { - return bytes.HasPrefix(y.ParseKey(it.Key()), prefix) +func hasPrefix(it *Iterator) bool { + if !it.opt.Reverse && len(it.opt.Prefix) > 0 { + return bytes.HasPrefix(y.ParseKey(it.iitr.Key()), it.opt.Prefix) } return true } @@ -741,7 +741,7 @@ func (it *Iterator) prefetch() { i := it.iitr var count int it.item = nil - for i.Valid() && hasPrefix(i, it.opt.Prefix) { + for i.Valid() && hasPrefix(it) { if !it.parseItem() { continue } From d0906d0b6df5e905045fedb80ff51c83984a73b4 Mon Sep 17 00:00:00 2001 From: Harshil Goel Date: Thu, 12 Sep 2024 11:42:03 +0530 Subject: [PATCH 2/4] fixed typo --- iterator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iterator.go b/iterator.go index 2232381bc..560a85c97 100644 --- a/iterator.go +++ b/iterator.go @@ -589,7 +589,7 @@ func (it *Iterator) Next() { // Set next item to current it.item = it.data.pop() - for it.iitr.Valid() && (!it.opt.Reverse && hasPrefix(it)) { + for it.iitr.Valid() && hasPrefix(it) { if it.parseItem() { // parseItem calls one extra next. // This is used to deal with the complexity of reverse iteration. From 1b76b2454dcfad1bae5e51d3b7ab2f4693e76f81 Mon Sep 17 00:00:00 2001 From: Harshil Goel Date: Thu, 12 Sep 2024 11:48:38 +0530 Subject: [PATCH 3/4] fixed comments --- iterator.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/iterator.go b/iterator.go index 560a85c97..8662a8593 100644 --- a/iterator.go +++ b/iterator.go @@ -726,6 +726,8 @@ func (it *Iterator) fill(item *Item) { } func hasPrefix(it *Iterator) bool { + // We shouldn't check prefix in case the iterator is going in reverse. Since in reverse we expect + // people to append items to the end of prefix. if !it.opt.Reverse && len(it.opt.Prefix) > 0 { return bytes.HasPrefix(y.ParseKey(it.iitr.Key()), it.opt.Prefix) } From eff4f481b910265ff305d650426072ec1808a54d Mon Sep 17 00:00:00 2001 From: Harshil Goel Date: Thu, 12 Sep 2024 12:14:10 +0530 Subject: [PATCH 4/4] Fixed lint --- db_test.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/db_test.go b/db_test.go index 5a535db4d..61ffe7c9d 100644 --- a/db_test.go +++ b/db_test.go @@ -167,17 +167,21 @@ func runBadgerTest(t *testing.T, opts *Options, test func(t *testing.T, db *DB)) func TestReverseIterator(t *testing.T) { runBadgerTest(t, nil, func(t *testing.T, db *DB) { key := make([]byte, 6) - db.Update(func(txn *Txn) error { + err := db.Update(func(txn *Txn) error { binary.BigEndian.PutUint16(key, 5) binary.BigEndian.PutUint32(key[2:], 1) - txn.Set(key, []byte("value1")) + err1 := txn.Set(key, []byte("value1")) + require.NoError(t, err1) binary.BigEndian.PutUint32(key[2:], 2) - txn.Set(key, []byte("value2")) + err1 = txn.Set(key, []byte("value2")) + require.NoError(t, err1) return nil }) - db.View(func(txn *Txn) error { + require.NoError(t, err) + + err = db.View(func(txn *Txn) error { searchBuffer := make([]byte, 3) binary.BigEndian.PutUint16(searchBuffer, 5) searchBuffer[2] = 0xFF @@ -193,6 +197,8 @@ func TestReverseIterator(t *testing.T) { require.Equal(t, it.Item().Key(), key) return nil }) + + require.NoError(t, err) }) }