diff --git a/iterator.go b/iterator.go index f57cfa4c9..52542f909 100644 --- a/iterator.go +++ b/iterator.go @@ -717,10 +717,23 @@ 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) + if it.opt.Reverse || len(it.opt.Prefix) == 0 { + return true } - return true + // iitr.Key() is the internal key = userKey + 8-byte ts. When len(Prefix) fits + // entirely within the userKey portion (len(key) >= len(Prefix)+8), then + // bytes.HasPrefix(internalKey, Prefix) is equivalent to + // bytes.HasPrefix(y.ParseKey(internalKey), Prefix) — the prefix can only + // match bytes that lie before the ts suffix. This elides one y.ParseKey + // (nil-check + sub + slice) per iterator Next on the hot dgraph rollup loop. + key := it.iitr.Key() + p := it.opt.Prefix + if len(key) >= len(p)+8 { + return bytes.HasPrefix(key, p) + } + // Prefix is longer than userKey — must reparse so we don't spuriously match + // against ts bytes. + return bytes.HasPrefix(y.ParseKey(key), p) } func (it *Iterator) prefetch() { diff --git a/iterator_hasprefix_test.go b/iterator_hasprefix_test.go new file mode 100644 index 000000000..3c07378be --- /dev/null +++ b/iterator_hasprefix_test.go @@ -0,0 +1,66 @@ +/* + * SPDX-FileCopyrightText: © Hypermode Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package badger + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestRegressionHasPrefixShortPrefixFallback covers the hasPrefix fallback +// branch where the user-supplied prefix is longer than userKey (len(p) > +// len(key)-8). The short-circuit "len(key) >= len(p)+8" must be false so we +// take the ParseKey path and correctly return false (no spurious match +// against ts bytes). +func TestRegressionHasPrefixShortPrefixFallback(t *testing.T) { + runBadgerTest(t, nil, func(t *testing.T, db *DB) { + // userKey "z" sorts AFTER prefix "yy", so Seek(prefix) lands on it + // and hasPrefix is actually called. The internal key is "z"+8 ts + // bytes (9 bytes), strictly shorter than prefix(2)+8=10, forcing + // the ParseKey fallback branch. + txnSet(t, db, []byte("z"), []byte("v"), 0) + + require.NoError(t, db.View(func(txn *Txn) error { + opt := DefaultIteratorOptions + opt.Prefix = []byte("yy") // 2 bytes, longer than userKey "z" + it := txn.NewIterator(opt) + defer it.Close() + count := 0 + for it.Rewind(); it.Valid(); it.Next() { + count++ + } + require.Equal(t, 0, count, "no key should match an over-long prefix") + return nil + })) + }) +} + +// TestRegressionHasPrefixFastPath covers the optimized hasPrefix path where +// len(prefix) fits within userKey: it must still correctly identify matching +// and non-matching keys, equivalent to the old ParseKey-based check. +func TestRegressionHasPrefixFastPath(t *testing.T) { + runBadgerTest(t, nil, func(t *testing.T, db *DB) { + txnSet(t, db, []byte("alpha/1"), []byte("v"), 0) + txnSet(t, db, []byte("alpha/2"), []byte("v"), 0) + txnSet(t, db, []byte("beta/1"), []byte("v"), 0) + + require.NoError(t, db.View(func(txn *Txn) error { + opt := DefaultIteratorOptions + opt.Prefix = []byte("alpha/") + it := txn.NewIterator(opt) + defer it.Close() + count := 0 + for it.Rewind(); it.Valid(); it.Next() { + require.True(t, bytes.HasPrefix(it.Item().Key(), opt.Prefix)) + count++ + } + require.Equal(t, 2, count, "alpha/ should match both alpha/1 and alpha/2") + return nil + })) + }) +}