Skip to content

Commit

Permalink
Fix bugs in scanLimit where the bounds are encompassing, but the exac…
Browse files Browse the repository at this point in the history 
…t values of those bounds do not exist

* fix for bug in reverse scanLimit that returned an additional element over the upperBound on the leaf node (data leak)
* fix for bug in forward scanLimit that returned an additional element over the upperBound on the leaf node (data leak)
* fix for bug in reverse scanLimit that returned 0 elements when starting iteration from an upper bound that is in-between the internal node and the leaf element directly after it
* Add additional unit tests to cover different leaf and internal cases with scanLimit
  • Loading branch information
@ByronBecker
ByronBecker committed Mar 8, 2023
1 parent d00c660 commit 5475ee9
Show file tree
Hide file tree
Showing 2 changed files with 217 additions and 5 deletions.
18 changes: 13 additions & 5 deletions src/BTree.mo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import O "mo:base/Order";
import Nat "mo:base/Nat";
import Stack "mo:base/Stack";


module {
public type BTree<K, V> = Types.BTree<K, V>;
public type Node<K, V> = Types.Node<K, V>;
Expand Down Expand Up @@ -698,7 +697,14 @@ module {
limit = 0;
nextKey = switch(data.kvs[elementIndex]) {
case null { null };
case (?kv) { ?kv.0 };
case (?kv) {
switch(compare(kv.0, upperBound)) {
// if the next key is greater than the upper bound, have hit the upper bound, so return null
case (#greater) { null };
// otherwise less or equal to the upper bound, so return the next key
case _ { ?kv.0 };
};
};
};
};
};
Expand All @@ -717,12 +723,14 @@ module {
if (idx == 0) {
return {
resultBuffer;
limit = 0;
limit = remainingLimit;
nextKey = null;
}
};

if (idx == data.count) { idx - 1: Nat } else { idx };
// We are iterating in reverse and did not find the upper bound, choose the previous element
// idx is not 0, so we can safely subtract 1
idx - 1: Nat;
};
};

Expand Down Expand Up @@ -812,7 +820,7 @@ module {

label l while (remainingLimit > 0) {
switch(nodeStack.pop()) {
case (?#leafCursor(leaf)) {
case (?#leafCursor(leaf)) {
let intermediateScanResult = iterScanLimitLeafForward(leaf, compare, lowerBound, upperBound, remainingLimit);
resultBuffer.append(intermediateScanResult.resultBuffer);
remainingLimit := intermediateScanResult.limit;
Expand Down
204 changes: 204 additions & 0 deletions test/BTreeTest.mo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2959,6 +2959,23 @@ let scanLimitSuite = S.suite("scanLimit", [
nextKey = ?3;
}))
),
S.test("if there are gaps in the contents of the BTree (i.e. every other number) and the limit is less than the result set",
do {
let t = quickCreateBTreeWithKVPairs(8, Array.tabulate<Nat>(4, func(i) { i*2+1 }));
BT.scanLimit(
t,
Nat.compare,
2,
8,
#fwd,
2,
)
},
M.equals(BTM.testableNatBTreeScanLimitResult({
results = [(3, 3), (5, 5)];
nextKey = ?7;
}))
),
]),
S.suite("with BTree as multiple levels", [
S.test("if lower bound is greater than the greatest key returns empty response",
Expand Down Expand Up @@ -3064,6 +3081,74 @@ let scanLimitSuite = S.suite("scanLimit", [
nextKey = null;
}))
),
S.test("if bounds are from the first to right before a root internal kv, there are gaps in the contents of the BTree (i.e. every other number), and the limit is equal to the result set",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(4, func(i) { i*2+1 }));
BT.scanLimit(
t,
Nat.compare,
0,
4,
#fwd,
2,
)
},
M.equals(BTM.testableNatBTreeScanLimitResult({
results = [(1, 1), (3, 3)];
nextKey = null;
}))
),
S.test("if bounds are from the first to right before a 3-level root internal kv, there are gaps in the contents of the BTree (i.e. every other number), and the limit is equal to the result set",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(13, func(i) { i*2+1 }));
BT.scanLimit(
t,
Nat.compare,
10,
16,
#fwd,
3,
)
},
M.equals(BTM.testableNatBTreeScanLimitResult({
results = [(11, 11), (13, 13), (15, 15)];
nextKey = null;
}))
),
S.test("if bounds are from the first to right before a first non-root internal kv, there are gaps in the contents of the BTree (i.e. every other number), and the limit is equal to the result set",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(13, func(i) { i*2+1 }));
BT.scanLimit(
t,
Nat.compare,
14,
22,
#fwd,
4,
)
},
M.equals(BTM.testableNatBTreeScanLimitResult({
results = [(15, 15), (17, 17), (19, 19), (21, 21)];
nextKey = null;
}))
),
S.test("if bounds are from the first to right before a last non-root internal kv, there are gaps in the contents of the BTree (i.e. every other number), and the limit is equal to the result set",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(13, func(i) { i*2+1 }));
BT.scanLimit(
t,
Nat.compare,
2,
10,
#fwd,
4,
)
},
M.equals(BTM.testableNatBTreeScanLimitResult({
results = [(3, 3), (5, 5), (7, 7), (9, 9)];
nextKey = null;
}))
),
S.test("if bounds are from the first to an root internal kv",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(31, func(i) { i+1 }));
Expand All @@ -3081,6 +3166,23 @@ let scanLimitSuite = S.suite("scanLimit", [
nextKey = null;
}))
),
S.test("if bounds are from the first to an root internal kv, there are gaps in the contents of the BTree (i.e. every other number), and the limit is equal to the result set",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(4, func(i) { i*2+1 }));
BT.scanLimit(
t,
Nat.compare,
0,
6,
#fwd,
3,
)
},
M.equals(BTM.testableNatBTreeScanLimitResult({
results = [(1, 1), (3, 3), (5, 5)];
nextKey = null;
}))
),
S.test("if bounds are from a middle leaf element to the last element",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(31, func(i) { i+1 }));
Expand Down Expand Up @@ -3394,6 +3496,23 @@ let scanLimitSuite = S.suite("scanLimit", [
nextKey = ?1;
}))
),
S.test("if there are gaps in the contents of the BTree (i.e. every other number) and the limit is less than the result set",
do {
let t = quickCreateBTreeWithKVPairs(8, Array.tabulate<Nat>(4, func(i) { i*2+1 }));
BT.scanLimit(
t,
Nat.compare,
0,
6,
#bwd,
2,
)
},
M.equals(BTM.testableNatBTreeScanLimitResult({
results = [(5, 5), (3, 3)];
nextKey = ?1;
}))
),
]),
S.suite("with BTree as multiple levels", [
S.test("if lower bound is greater than the greatest key returns empty response",
Expand Down Expand Up @@ -3499,6 +3618,23 @@ let scanLimitSuite = S.suite("scanLimit", [
nextKey = null;
}))
),
S.test("if bounds are from the last element to right before a root internal kv, there are gaps in the contents of the BTree (i.e. every other number), and the limit is equal to the result set",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(5, func(i) { i*2+1 }));
BT.scanLimit(
t,
Nat.compare,
6,
10,
#bwd,
2,
)
},
M.equals(BTM.testableNatBTreeScanLimitResult({
results = [(9, 9), (7, 7)];
nextKey = null;
}))
),
S.test("if bounds are from the first to an root internal kv",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(31, func(i) { i+1 }));
Expand All @@ -3516,6 +3652,74 @@ let scanLimitSuite = S.suite("scanLimit", [
nextKey = null;
}))
),
S.test("if bounds are from the first to right before a 3-level root internal kv, there are gaps in the contents of the BTree (i.e. every other number), and the limit is equal to the result set",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(13, func(i) { i*2+1 }));
BT.scanLimit(
t,
Nat.compare,
18,
24,
#bwd,
3,
)
},
M.equals(BTM.testableNatBTreeScanLimitResult({
results = [(23, 23), (21, 21), (19, 19)];
nextKey = null;
}))
),
S.test("if bounds are from the first to right before a first non-root internal kv, there are gaps in the contents of the BTree (i.e. every other number), and the limit is equal to the result set",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(13, func(i) { i*2+1 }));
BT.scanLimit(
t,
Nat.compare,
12,
20,
#bwd,
4,
)
},
M.equals(BTM.testableNatBTreeScanLimitResult({
results = [(19, 19), (17, 17), (15, 15), (13, 13)];
nextKey = null;
}))
),
S.test("if bounds are from the first to right before a last non-root internal kv, there are gaps in the contents of the BTree (i.e. every other number), and the limit is equal to the result set",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(13, func(i) { i*2+1 }));
BT.scanLimit(
t,
Nat.compare,
6,
14,
#bwd,
4,
)
},
M.equals(BTM.testableNatBTreeScanLimitResult({
results = [(13, 13), (11, 11), (9, 9), (7, 7)];
nextKey = null;
}))
),
S.test("if bounds are from the first to an root internal kv, there are gaps in the contents of the BTree (i.e. every other number), and the limit is less than the result set",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(4, func(i) { i*2+1 }));
BT.scanLimit(
t,
Nat.compare,
0,
6,
#bwd,
2,
)
},
M.equals(BTM.testableNatBTreeScanLimitResult({
results = [(5, 5), (3, 3)];
nextKey = ?1;
}))
),
S.test("if bounds are from a middle leaf element to the last element",
do {
let t = quickCreateBTreeWithKVPairs(4, Array.tabulate<Nat>(31, func(i) { i+1 }));
Expand Down

0 comments on commit 5475ee9

Please sign in to comment.