Skip to content

Commit

Permalink
Only delete row from ART if row id matches, not just if value matches
Browse files Browse the repository at this point in the history
  • Loading branch information
Mytherin committed Apr 11, 2020
1 parent 4cd2c70 commit 0d29535
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 16 deletions.
16 changes: 8 additions & 8 deletions src/execution/index/art/art.cpp
Expand Up @@ -347,8 +347,6 @@ void ART::Delete(IndexLock &state, DataChunk &input, Vector &row_ids) {
continue;
}
Erase(tree, *keys[i], 0, row_identifiers[i]);
// assert that the entry was erased properly
assert(!is_unique || Lookup(tree, *keys[i], 0) == nullptr);
}
}

Expand All @@ -360,7 +358,11 @@ void ART::Erase(unique_ptr<Node> &node, Key &key, unsigned depth, row_t row_id)
if (node->type == NodeType::NLeaf) {
// Make sure we have the right leaf
if (ART::LeafMatches(node.get(), key, depth)) {
node.reset();
auto leaf = static_cast<Leaf *>(node.get());
leaf->Remove(row_id);
if (leaf->num_elements == 0) {
node.reset();
}
}
return;
}
Expand All @@ -381,11 +383,9 @@ void ART::Erase(unique_ptr<Node> &node, Key &key, unsigned depth, row_t row_id)
if (child_ref->type == NodeType::NLeaf && LeafMatches(child_ref.get(), key, depth)) {
// Leaf found, remove entry
auto leaf = static_cast<Leaf *>(child_ref.get());
if (leaf->num_elements > 1) {
// leaf has multiple rows: remove the row from the leaf
leaf->Remove(row_id);
} else {
// Leaf only has one element, delete leaf, decrement node counter and maybe shrink node
leaf->Remove(row_id);
if (leaf->num_elements == 0) {
// Leaf is empty, delete leaf, decrement node counter and maybe shrink node
Node::Erase(*this, node, pos);
}
} else {
Expand Down
5 changes: 4 additions & 1 deletion src/execution/index/art/leaf.cpp
Expand Up @@ -27,13 +27,16 @@ void Leaf::Insert(row_t row_id) {

//! TODO: Maybe shrink array dynamically?
void Leaf::Remove(row_t row_id) {
idx_t entry_offset = -1;
idx_t entry_offset = INVALID_INDEX;
for (idx_t i = 0; i < num_elements; i++) {
if (row_ids[i] == row_id) {
entry_offset = i;
break;
}
}
if (entry_offset == INVALID_INDEX) {
return;
}
num_elements--;
for (idx_t j = entry_offset; j < num_elements; j++) {
row_ids[j] = row_ids[j + 1];
Expand Down
21 changes: 14 additions & 7 deletions test/rigger/test_rigger.cpp
Expand Up @@ -255,13 +255,13 @@ TEST_CASE("Tests found by Rigger", "[rigger]") {
}
SECTION("513") {
// LEFT JOIN with comparison on integer columns results in "Not implemented: Unimplemented type for nested loop join!"
// REQUIRE_NO_FAIL(con.Query("CREATE TABLE t0(c0 INT);"));
// REQUIRE_NO_FAIL(con.Query("CREATE TABLE t1(c0 INT);"));
// REQUIRE_NO_FAIL(con.Query("INSERT INTO t1(c0) VALUES (0);"));
// REQUIRE_NO_FAIL(con.Query("INSERT INTO t0(c0) VALUES (0);"));
// result = con.Query("SELECT * FROM t0 LEFT JOIN t1 ON t0.c0 <= t1.c0;");
// REQUIRE(CHECK_COLUMN(result, 0, {0}));
// REQUIRE(CHECK_COLUMN(result, 1, {0}));
REQUIRE_NO_FAIL(con.Query("CREATE TABLE t0(c0 INT);"));
REQUIRE_NO_FAIL(con.Query("CREATE TABLE t1(c0 INT);"));
REQUIRE_NO_FAIL(con.Query("INSERT INTO t1(c0) VALUES (0);"));
REQUIRE_NO_FAIL(con.Query("INSERT INTO t0(c0) VALUES (0);"));
result = con.Query("SELECT * FROM t0 LEFT JOIN t1 ON t0.c0 <= t1.c0;");
REQUIRE(CHECK_COLUMN(result, 0, {0}));
REQUIRE(CHECK_COLUMN(result, 1, {0}));
}
SECTION("514") {
// Incorrect result after an INSERT violates a UNIQUE constraint
Expand All @@ -273,6 +273,13 @@ TEST_CASE("Tests found by Rigger", "[rigger]") {
REQUIRE_FAIL(con.Query("INSERT INTO t0(c0) VALUES (1);"));
result = con.Query("SELECT * FROM t0 WHERE t0.c0 = 1;");
REQUIRE(CHECK_COLUMN(result, 0, {1}));

// verify correct behavior here too when we have multiple nodes
REQUIRE_NO_FAIL(con.Query("INSERT INTO t0(c0) VALUES (2);"));
REQUIRE_NO_FAIL(con.Query("INSERT INTO t0(c0) VALUES (3);"));
REQUIRE_FAIL(con.Query("INSERT INTO t0(c0) VALUES (2);"));
result = con.Query("SELECT * FROM t0 WHERE t0.c0 = 2;");
REQUIRE(CHECK_COLUMN(result, 0, {2}));
}
SECTION("515") {
// Query with a negative shift predicate yields an incorrect result
Expand Down

0 comments on commit 0d29535

Please sign in to comment.