SimpleDeleteTest in executor_test.cpp incorrectly checks if index does not have removed row anymore.

[adaptun]

SimpleDeleteTest in executor_test.cpp checks if index does not have removed row anymore.

See lines 359-372:

const Tuple index_key = Tuple(result_set[0]);
std::unique_ptr<AbstractPlanNode> delete_plan;
{ delete_plan = std::make_unique<DeletePlanNode>(scan_plan1.get(), table_info->oid_); }
GetExecutionEngine()->Execute(delete_plan.get(), nullptr, GetTxn(), GetExecutorContext());

result_set.clear();

// SELECT col_a FROM test_1 WHERE col_a == 50
GetExecutionEngine()->Execute(scan_plan1.get(), &result_set, GetTxn(), GetExecutorContext());
ASSERT_TRUE(result_set.empty());

// Ensure the key was removed from the index
std::vector<RID> rids{};
index_info->index_->ScanKey(index_key, &rids, GetTxn());

I think problem here is that tuple index_key is taken from the resultset and is not converted to the index tuple.

Should be something as the following:

auto scan_key = index_key.KeyFromTuple(GetExecutorContext()->GetCatalog()->GetTable("test_1")->schema_,
                                           index_info->key_schema_, index_info->index_->GetKeyAttrs());

index_info->index_->ScanKey(scan_key, &rids, GetTxn());

Compare with SimpleRawInsertWithIndexTest where tuple from resultset is converted to the index_key -- line 260.

bustub/test/execution/executor_test.cpp

Line 372 in 6efd294

index_info->index_->ScanKey(index_key, &rids, GetTxn());

[turingcompl33t]

Hey @adaptun you are correct, this is an oversight in the test and we should convert the tuple from the result set to an index key properly before performing the scan. In this particular case, this step is not strictly necessary for correctness because the schema of the index key and the output schema of the scan are identical, but in any other scenario this could lead to incorrect behavior. This project is currently ongoing in our databases course so we'll hold off from making this update for now, but once the academic term is over this is an issue that we will address.

[skyzh]

We'll deprecate executor test (unit test) this semester. Won't fix.