Skip to content

Commit

Permalink
atlas: fix bug in PrefixTree (#1167)
Browse files Browse the repository at this point in the history
If a node only had a single child after a removal, it would
get compressed by merging the prefix. However the children
of the merged child node were getting dropped.
  • Loading branch information
brharrington authored Oct 17, 2024
1 parent 17e6d74 commit 076b4f6
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ Node compress() {
} else if (inQueries.isEmpty() && otherQueries.isEmpty() && cs.size() == 1) {
Node c = cs.get(0);
String p = prefix + c.prefix;
return new Node(p, EMPTY, c.inQueries, c.otherQueries);
return new Node(p, c.children, c.inQueries, c.otherQueries);
} else {
return new Node(prefix, cs.toArray(EMPTY), inQueries, otherQueries);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,31 @@ public void manyDifferentRootPrefixes() {
assertEquals(list("abc"), tree, "abcdef");
}

@Test
public void compressWithSingleChildHavingNoDirectMatches() {
PrefixTree tree = new PrefixTree();
tree.put(re("abcdef"));
tree.put(re("abcghi"));
tree.put(re("xyz"));
Assertions.assertEquals(3, tree.size());

tree.remove(re("xyz"));
Assertions.assertEquals(2, tree.size());
}

@Test
public void compressWithSingleChildHavingDirectMatches() {
PrefixTree tree = new PrefixTree();
tree.put(re("abc"));
tree.put(re("abcdef"));
tree.put(re("abcghi"));
tree.put(re("xyz"));
Assertions.assertEquals(4, tree.size());

tree.remove(re("xyz"));
Assertions.assertEquals(3, tree.size());
}

@Test
public void commonPrefixNoMatch() {
Assertions.assertEquals(0, PrefixTree.commonPrefixLength("abcdef", "defghi", 0));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -636,4 +636,34 @@ public void updateRaceCondition() throws Exception {
}
});
}

@Test
public void otherChecksBug() {
Query q1 = Parser.parseQuery("name,foo,:eq,path,abcdef,:re,:and");
Query q2 = Parser.parseQuery("name,foo,:eq,path,abcghi,:re,:and");
Query q3 = Parser.parseQuery("name,foo,:eq,path,xyz,:re,:and");

QueryIndex<Integer> idx = QueryIndex.newInstance(new NoopRegistry());
idx.add(q1, 1);
idx.add(q2, 2);
idx.add(q3, 3);
Assertions.assertEquals(
Collections.singletonList(1),
idx.findMatches(id("foo", "path", "abcdef")));
Assertions.assertEquals(
Collections.singletonList(2),
idx.findMatches(id("foo", "path", "abcghijkl")));
Assertions.assertEquals(
Collections.singletonList(3),
idx.findMatches(id("foo", "path", "xyz")));

idx.remove(q3, 3);
Assertions.assertEquals(
Collections.singletonList(1),
idx.findMatches(id("foo", "path", "abcdef")));
Assertions.assertEquals(
Collections.singletonList(2),
idx.findMatches(id("foo", "path", "abcghijkl")));
Assertions.assertTrue(idx.findMatches(id("foo", "path", "xyz")).isEmpty());
}
}

0 comments on commit 076b4f6

Please sign in to comment.