Skip to content
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.

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

Fix SeekLowerBound where tree contains keys that are prefixes of each other #39

Merged
merged 9 commits into from
Jun 28, 2021
33 changes: 28 additions & 5 deletions iradix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1665,6 +1665,31 @@ func TestIterateLowerBound(t *testing.T) {
"cbacb",
[]string{"cbbaa", "cbbab", "cbbbc", "cbcbb", "cbcbc", "cbcca", "ccaaa", "ccabc", "ccaca", "ccacc", "ccbac", "cccaa", "cccac", "cccca"},
},

// We SHOULD support keys that are prefixes of each other despite some
// confusion in the original implementation.
{
[]string{"f", "fo", "foo", "food", "bug"},
"foo",
[]string{"foo", "food"},
},

// We also support the empty key as a valid key to insert and search for.
{
[]string{"f", "fo", "foo", "food", "bug", ""},
"foo",
[]string{"foo", "food"},
},
{
[]string{"f", "bug", ""},
"",
[]string{"", "bug", "f"},
},
{
[]string{"f", "bug", "xylophone"},
"",
[]string{"bug", "f", "xylophone"},
},
}

for idx, test := range cases {
Expand Down Expand Up @@ -1725,17 +1750,15 @@ func TestIterateLowerBoundFuzz(t *testing.T) {
set := []string{}

// This specifies a property where each call adds a new random key to the radix
// tree (with a null byte appended since our tree doesn't support one key
// being a prefix of another and treats null bytes specially).
// tree.
//
// It also maintains a plain sorted list of the same set of keys and asserts
// that iterating from some random key to the end using LowerBound produces
// the same list as filtering all sorted keys that are lower.

radixAddAndScan := func(newKey, searchKey readableString) []string {
// Append a null byte
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the comment also needs to be removed

Copy link
Contributor

Choose a reason for hiding this comment

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

oh! think it's removed in the second commit 🤦

key := []byte(newKey + "\x00")
r, _, _ = r.Insert(key, nil)
r, _, _ = r.Insert([]byte(newKey), nil)

// Now iterate the tree from searchKey to the end
it := r.Root().Iterator()
Expand All @@ -1747,7 +1770,7 @@ func TestIterateLowerBoundFuzz(t *testing.T) {
break
}
// Strip the null byte and append to result set
result = append(result, string(key[0:len(key)-1]))
result = append(result, string(key))
}
return result
}
Expand Down
8 changes: 2 additions & 6 deletions iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,8 @@ func (i *Iterator) SeekLowerBound(key []byte) {
}

// Prefix is equal, we are still heading for an exact match. If this is a
// leaf we're done.
if n.leaf != nil {
if bytes.Compare(n.leaf.key, key) < 0 {
i.node = nil
return
}
// leaf and an exact match we're done.
if n.leaf != nil && bytes.Equal(n.leaf.key, key) {
found(n)
return
}
Expand Down