Skip to content

Add key-only ScanKeys interface #234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bootjp wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add key-only ScanKeys interface #234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5dc7ec2
Refactor memory store ScanKeys implementation
bootjp Sep 13, 2025
67e1e50
Merge branch 'main' into codex/fix-issue-with-elastickv-feature-rrq5bv
bootjp Sep 13, 2025
77b6072
Merge branch 'main' into codex/fix-issue-with-elastickv-feature-rrq5bv
bootjp Dec 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions store/bolt_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,34 @@ func (s *boltStore) Scan(ctx context.Context, start []byte, end []byte, limit in
return res, errors.WithStack(err)
}

func (s *boltStore) ScanKeys(ctx context.Context, start []byte, end []byte, limit int) ([][]byte, error) {
s.log.InfoContext(ctx, "ScanKeys",
slog.String("start", string(start)),
slog.String("end", string(end)),
slog.Int("limit", limit),
)

var res [][]byte

err := s.bbolt.View(func(tx *bbolt.Tx) error {
b := tx.Bucket(defaultBucket)
if b == nil {
return nil
}

c := b.Cursor()
for k, _ := c.Seek(start); k != nil && (end == nil || bytes.Compare(k, end) < 0); k, _ = c.Next() {
res = append(res, k)
Copy link

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The BoltDB cursor returns slices that reference the underlying database memory. These keys need to be copied to prevent data corruption when the transaction ends, similar to how the memory store implementation copies keys.

Copilot uses AI. Check for mistakes.
Copy link

@gemini-code-assist gemini-code-assist bot Sep 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The key k obtained from the BoltDB cursor points to memory that is only valid for the lifetime of the transaction. By appending k directly to the result slice, you are returning a slice that points to invalid memory after the transaction completes. This can lead to data corruption or application crashes. You must create a copy of the key before appending it to the result slice. 1

            keyCopy := make([]byte, len(k))
            copy(keyCopy, k)
            res = append(res, keyCopy)

Style Guide References

Footnotes

  1. The BoltDB documentation for Cursor.Seek and Cursor.Next states: 'The returned key and value are only valid for the life of the transaction.' It is crucial to copy the key and value if they need to be used outside the transaction. ↩

if len(res) >= limit {
Comment on lines +113 to +116
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Sep 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Copy Bolt scan keys before returning them

The Bolt implementation of ScanKeys appends the cursor’s k slice directly into the result. Bolt’s cursor reuses these byte slices and documentation states they are only valid for the lifetime of the transaction; after the View callback returns the slices may point at corrupted data. Callers using the returned keys outside the transaction will observe mutated keys or crashes. Consider copying each key into a new slice (as the in-memory implementation and TestRbMemoryStore_ScanKeysReturnsCopies expect) before appending.

Useful? React with 👍 / 👎.

break
}
}
return nil
})

return res, errors.WithStack(err)
}

func (s *boltStore) Put(ctx context.Context, key []byte, value []byte) error {
s.log.InfoContext(ctx, "put",
slog.String("key", string(key)),
Expand Down
39 changes: 39 additions & 0 deletions store/bolt_store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,45 @@ func TestBoltStore_Scan(t *testing.T) {
assert.Equal(t, 100, cnt)
}

func TestBoltStore_ScanKeys(t *testing.T) {
ctx := context.Background()
t.Parallel()
st := mustStore(NewBoltStore(t.TempDir() + "/bolt.db"))

for i := 0; i < 999; i++ {
keyStr := "prefix " + strconv.Itoa(i) + "foo"
key := []byte(keyStr)
b := make([]byte, 8)
binary.PutVarint(b, int64(i))
err := st.Put(ctx, key, b)
assert.NoError(t, err)
}

res, err := st.ScanKeys(ctx, []byte("prefix"), []byte("z"), 100)
assert.NoError(t, err)
assert.Equal(t, 100, len(res))

sortedKeys := make([][]byte, 999)

for _, k := range res {
str := string(k)
i, err := strconv.Atoi(str[7 : len(str)-3])
assert.NoError(t, err)
sortedKeys[i] = k
}

cnt := 0
for i, k := range sortedKeys {
if k == nil {
continue
}
cnt++
assert.Equal(t, []byte("prefix "+strconv.Itoa(i)+"foo"), k)
}

assert.Equal(t, 100, cnt)
}

func TestBoltStore_Txn(t *testing.T) {
t.Parallel()
t.Run("success", func(t *testing.T) {
Expand Down
34 changes: 34 additions & 0 deletions store/rb_memory_store.go
View file
Open in desktop
Copy link

@github-actions github-actions bot Dec 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
Magic number: 4, in detected (mnd)

elastickv/store/rb_memory_store.go

Line 287 in 77b6072

if len(data) < 4 {

Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,40 @@ func (s *rbMemoryStore) Scan(ctx context.Context, start []byte, end []byte, limi
return result, nil
}

func (s *rbMemoryStore) ScanKeys(ctx context.Context, start []byte, end []byte, limit int) ([][]byte, error) {
s.mtx.RLock()
defer s.mtx.RUnlock()

var result [][]byte

it := s.tree.Iterator()

var ok bool
if start != nil {
it.Begin()
ok = it.NextTo(func(key, _ interface{}) bool {
k, _ := key.([]byte)
return bytes.Compare(k, start) >= 0
})
} else {
ok = it.First()
}
Comment on lines +140 to +148
Copy link

@gemini-code-assist gemini-code-assist bot Sep 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The use of it.NextTo to find the starting position for the scan results in a linear scan from the beginning of the tree. For a large number of keys, this is inefficient with a time complexity of O(N). A range scan on a balanced tree should ideally have a seek time of O(log N).

While the gods/treemap iterator may not have a more efficient public method for seeking, this implementation negates the performance benefit of using a tree for range scans. Please consider if there's a more performant way to achieve this with the current library, or at least document this performance characteristic.


for ; ok && len(result) < limit; ok = it.Next() {
k, _ := it.Key().([]byte)
Copy link

@gemini-code-assist gemini-code-assist bot Sep 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The blank identifier _ is redundant in this type assertion. When a type assertion has a single return value, it will panic on failure. Using k, _ := ... is stylistically the same as k := ... but less clear. If you want to handle a potential type mismatch gracefully, you should check the second boolean return value (ok). Given that keys in this map are always []byte, a single-value assertion is appropriate and more idiomatic.

Suggested change
k, _ := it.Key().([]byte)
k := it.Key().([]byte)


if end != nil && bytes.Compare(k, end) > 0 {
break
}

keyCopy := make([]byte, len(k))
copy(keyCopy, k)
result = append(result, keyCopy)
}

return result, nil
}

func (s *rbMemoryStore) Put(ctx context.Context, key []byte, value []byte) error {
s.mtx.Lock()
defer s.mtx.Unlock()
Expand Down
58 changes: 58 additions & 0 deletions store/rb_memory_store_test.go
View file
Open in desktop
Copy link

@github-actions github-actions bot Dec 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
right hand must be only type assertion (forcetypeassert)

elastickv/store/rb_memory_store_test.go

Line 280 in 77b6072

err = st2.Restore(bytes.NewReader(buf.(*bytes.Buffer).Bytes()))

Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,64 @@ func TestRbMemoryStore_Scan(t *testing.T) {
assert.Equal(t, 100, cnt)
}

func TestRbMemoryStore_ScanKeys(t *testing.T) {
ctx := context.Background()
t.Parallel()
st := NewRbMemoryStore()

for i := 0; i < 9999; i++ {
keyStr := "prefix " + strconv.Itoa(i) + "foo"
key := []byte(keyStr)
b := make([]byte, 8)
binary.PutVarint(b, int64(i))
err := st.Put(ctx, key, b)
assert.NoError(t, err)
}

res, err := st.ScanKeys(ctx, []byte("prefix"), []byte("z"), 100)
assert.NoError(t, err)
assert.Equal(t, 100, len(res))

sortedKeys := make([][]byte, 9999)

for _, k := range res {
str := string(k)
i, err := strconv.Atoi(str[7 : len(str)-3])
assert.NoError(t, err)
sortedKeys[i] = k
}

cnt := 0
for i, k := range sortedKeys {
if k == nil {
continue
}
cnt++
assert.Equal(t, []byte("prefix "+strconv.Itoa(i)+"foo"), k)
}

assert.Equal(t, 100, cnt)
}

func TestRbMemoryStore_ScanKeysReturnsCopies(t *testing.T) {
ctx := context.Background()
st := NewRbMemoryStore()

assert.NoError(t, st.Put(ctx, []byte("foo"), []byte("bar")))

res, err := st.ScanKeys(ctx, nil, nil, 10)
assert.NoError(t, err)
if len(res) == 0 {
t.Fatalf("expected keys, got none")
}

res[0][0] = 'x'

res2, err := st.ScanKeys(ctx, nil, nil, 10)
assert.NoError(t, err)
assert.Equal(t, []byte("foo"), res2[0])
}

func TestRbMemoryStore_Txn(t *testing.T) {
t.Parallel()
t.Run("success", func(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions store/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type Store interface {
type ScanStore interface {
Store
Scan(ctx context.Context, start []byte, end []byte, limit int) ([]*KVPair, error)
ScanKeys(ctx context.Context, start []byte, end []byte, limit int) ([][]byte, error)
}

type TTLStore interface {
Expand All @@ -57,6 +58,7 @@ type Txn interface {
type ScanTxn interface {
Txn
Scan(ctx context.Context, start []byte, end []byte, limit int) ([]*KVPair, error)
ScanKeys(ctx context.Context, start []byte, end []byte, limit int) ([][]byte, error)
}

type TTLTxn interface {
Expand Down
Loading