From d2091a3bede633937f74b8c1e0ecdf0c436fa240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Poyraz=20K=C3=BC=C3=A7=C3=BCkarslan?= <83272398+PoyrazK@users.noreply.github.com> Date: Thu, 30 Apr 2026 16:01:41 +0300 Subject: [PATCH 1/3] test: add branch coverage tests for transaction_manager, operator, query_executor - transaction_manager: add tests for DELETE table not found, UPDATE physical_remove failure, UPDATE undo_remove failure - operator: add tests for next_view() branches on ProjectOperator, LimitOperator, and FilterOperator - query_executor: add batch_insert_mode test --- tests/operator_tests.cpp | 142 ++++++++++++++++++++++++++++ tests/query_executor_tests.cpp | 28 ++++++ tests/transaction_manager_tests.cpp | 104 ++++++++++++++++++++ 3 files changed, 274 insertions(+) diff --git a/tests/operator_tests.cpp b/tests/operator_tests.cpp index c955f31..3934dcf 100644 --- a/tests/operator_tests.cpp +++ b/tests/operator_tests.cpp @@ -1252,4 +1252,146 @@ TEST_F(OperatorTests, HashJoinNullKeys) { join->close(); } +TEST_F(OperatorTests, ProjectNextViewNonSimpleProjection) { + // Test ProjectOperator::next_view() with non-simple projection (!is_simple_projection_) + // next_view() returns false early when is_simple_projection_ is false + // (BufferScanOperator doesn't implement next_view(), so we test the early-return path) + Schema schema = make_schema( + {{"id", common::ValueType::TYPE_INT64}, {"name", common::ValueType::TYPE_TEXT}}); + std::vector data; + data.push_back(make_tuple({common::Value::make_int64(1), common::Value::make_text("alice")})); + + auto scan = make_buffer_scan("test_table", data, schema); + + // Use a constant expression instead of column reference — this makes is_simple_projection_ = false + std::vector> cols; + cols.push_back(const_expr(common::Value::make_int64(42))); // constant, not column + auto project = make_project(std::move(scan), std::move(cols)); + + ASSERT_TRUE(project->init()); + ASSERT_TRUE(project->open()); + + // next() should work (materializes the constant) + Tuple tuple; + EXPECT_TRUE(project->next(tuple)); + EXPECT_EQ(tuple.size(), 1U); + + // next_view() returns false early for non-simple projection + storage::HeapTable::TupleView view; + EXPECT_FALSE(project->next_view(view)); + + project->close(); +} + +TEST_F(OperatorTests, LimitNextView) { + // Test LimitOperator::next_view() — BufferScanOperator doesn't implement next_view() + // (uses base stub that returns false), so we test the early-return paths + Schema schema = make_schema({{"id", common::ValueType::TYPE_INT64}}); + std::vector data; + for (int i = 0; i < 5; i++) { + data.push_back(make_tuple({common::Value::make_int64(i)})); + } + + auto scan = make_buffer_scan("test_table", data, schema); + auto limit = make_limit(std::move(scan), 2); // limit 2 + + ASSERT_TRUE(limit->init()); + ASSERT_TRUE(limit->open()); + + // LimitOperator::next_view() calls child_->next_view() which returns false + // (BufferScan doesn't implement next_view). This exercises the early-return path. + storage::HeapTable::TupleView view; + EXPECT_FALSE(limit->next_view(view)); + limit->close(); +} + +TEST_F(OperatorTests, LimitNextViewZeroLimit) { + // Test LimitOperator::next_view() with limit=0 + Schema schema = make_schema({{"id", common::ValueType::TYPE_INT64}}); + std::vector data; + for (int i = 0; i < 5; i++) { + data.push_back(make_tuple({common::Value::make_int64(i)})); + } + + auto scan = make_buffer_scan("test_table", data, schema); + auto limit = make_limit(std::move(scan), 0); // limit 0 + + ASSERT_TRUE(limit->init()); + ASSERT_TRUE(limit->open()); + + // With limit=0, next_view() returns false early + // This exercises the limit check + storage::HeapTable::TupleView view; + EXPECT_FALSE(limit->next_view(view)); + limit->close(); +} + +TEST_F(OperatorTests, LimitNextViewOffsetExceedsTotal) { + // Test LimitOperator::next_view() when offset exceeds data size + Schema schema = make_schema({{"id", common::ValueType::TYPE_INT64}}); + std::vector data; + for (int i = 0; i < 3; i++) { + data.push_back(make_tuple({common::Value::make_int64(i)})); + } + + auto scan = make_buffer_scan("test_table", data, schema); + auto limit = make_limit(std::move(scan), 10, 100); // offset 100, limit 10 + + ASSERT_TRUE(limit->init()); + ASSERT_TRUE(limit->open()); + + // offset 100 exceeds data size 3, so child_->next_view() returns false + // This exercises the offset loop early-return + storage::HeapTable::TupleView view; + EXPECT_FALSE(limit->next_view(view)); + limit->close(); +} + +TEST_F(OperatorTests, FilterNextViewChildReturnsFalse) { + // Test FilterOperator::next_view() when child returns false + // BufferScanOperator doesn't implement next_view(), so child_->next_view() returns false + // This exercises the child returns false path + Schema schema = make_schema({{"id", common::ValueType::TYPE_INT64}}); + std::vector data; + for (int i = 0; i < 5; i++) { + data.push_back(make_tuple({common::Value::make_int64(i)})); + } + + auto scan = make_buffer_scan("test_table", data, schema); + auto filter = make_filter( + std::move(scan), + binary_expr(col_expr("id"), TokenType::Ge, const_expr(common::Value::make_int64(2)))); + + ASSERT_TRUE(filter->init()); + ASSERT_TRUE(filter->open()); + + storage::HeapTable::TupleView view; + EXPECT_FALSE(filter->next_view(view)); + filter->close(); +} + +TEST_F(OperatorTests, FilterNextViewAllFiltered) { + // Test FilterOperator::next_view() when all tuples are filtered out + // This exercises the condition evaluate and loop iteration path + Schema schema = make_schema({{"id", common::ValueType::TYPE_INT64}}); + std::vector data; + for (int i = 0; i < 3; i++) { + data.push_back(make_tuple({common::Value::make_int64(i)})); + } + + auto scan = make_buffer_scan("test_table", data, schema); + // Filter: id > 100 (filters all) + auto filter = make_filter( + std::move(scan), + binary_expr(col_expr("id"), TokenType::Gt, const_expr(common::Value::make_int64(100)))); + + ASSERT_TRUE(filter->init()); + ASSERT_TRUE(filter->open()); + + storage::HeapTable::TupleView view; + // BufferScan next_view returns false immediately, so we never even get to evaluate condition + EXPECT_FALSE(filter->next_view(view)); + filter->close(); +} + } // namespace diff --git a/tests/query_executor_tests.cpp b/tests/query_executor_tests.cpp index 4f2e5f6..f829930 100644 --- a/tests/query_executor_tests.cpp +++ b/tests/query_executor_tests.cpp @@ -185,6 +185,34 @@ TEST_F(QueryExecutorTests, InsertIntoNonExistentTable) { EXPECT_FALSE(res.success()); } +TEST_F(QueryExecutorTests, InsertBatchModeSkipsLockAcquisition) { + // Test batch_insert_mode=true skips lock acquisition (line 217) + TestEnvironment env; + execute_sql(env.executor, "CREATE TABLE test_table (id INT, val INT)"); + + // Enable batch insert mode - skips lock acquisition per line 217 + env.executor.set_batch_insert_mode(true); + + // BEGIN transaction + execute_sql(env.executor, "BEGIN"); + + // Multi-row INSERT - should succeed without lock acquisition + const auto res = execute_sql( + env.executor, "INSERT INTO test_table VALUES (1, 10), (2, 20), (3, 30)"); + EXPECT_TRUE(res.success()); + EXPECT_EQ(res.rows_affected(), 3U); + + // COMMIT + execute_sql(env.executor, "COMMIT"); + + // Verify data was inserted + const auto select_res = execute_sql(env.executor, "SELECT * FROM test_table"); + EXPECT_EQ(select_res.row_count(), 3U); + + // Cleanup + env.executor.set_batch_insert_mode(false); +} + // ============= SELECT Tests ============= TEST_F(QueryExecutorTests, SelectStarFromEmptyTable) { diff --git a/tests/transaction_manager_tests.cpp b/tests/transaction_manager_tests.cpp index 899083b..44e9064 100644 --- a/tests/transaction_manager_tests.cpp +++ b/tests/transaction_manager_tests.cpp @@ -1508,4 +1508,108 @@ TEST(TransactionManagerTests, UndoLogUnknownType) { static_cast(std::remove("./test_data/unknown_test.heap")); } +TEST(TransactionManagerTests, UndoDeleteTableNotFound) { + // Test table metadata not found branch in DELETE undo path + // Manually add DELETE undo log for non-existent table, call abort() + auto catalog = Catalog::create(); + storage::StorageManager disk_manager("./test_data"); + storage::BufferPoolManager bpm(cloudsql::config::Config::DEFAULT_BUFFER_POOL_SIZE, + disk_manager); + LockManager lm; + TransactionManager tm(lm, *catalog, bpm, bpm.get_log_manager()); + + Transaction* txn = tm.begin(); + ASSERT_NE(txn, nullptr); + + // Add DELETE undo log for non-existent table + txn->add_undo_log(UndoLog::Type::DELETE, "nonexistent_delete_table", + HeapTable::TupleId(99, 99)); + + // abort() should hit table metadata lookup failure + tm.abort(txn); + EXPECT_EQ(txn->get_state(), TransactionState::ABORTED); +} + +TEST(TransactionManagerTests, UndoUpdatePhysicalRemoveFailure) { + // Test FAULT_PHYSICAL_REMOVE branch in UPDATE undo + storage::StorageManager disk_manager("./test_data"); + disk_manager.create_dir_if_not_exists(); + recovery::LogManager log_mgr("./test_data/upd_phys_rm_fault.dat"); + storage::BufferPoolManager bpm(cloudsql::config::Config::DEFAULT_BUFFER_POOL_SIZE, disk_manager, + &log_mgr); + auto catalog = Catalog::create(); + LockManager lm; + TransactionManager tm(lm, *catalog, bpm, &log_mgr); + executor::QueryExecutor exec(*catalog, bpm, lm, tm); + + static_cast(std::remove("./test_data/upd_phys_rm.heap")); + static_cast(std::remove("./test_data/upd_phys_rm.idx")); + + static_cast( + exec.execute(*Parser(std::make_unique("CREATE TABLE upd_phys_rm (id INT, val INT)")) + .parse_statement())); + static_cast( + exec.execute(*Parser(std::make_unique("CREATE INDEX idx_upr ON upd_phys_rm (val)")) + .parse_statement())); + static_cast( + exec.execute(*Parser(std::make_unique("INSERT INTO upd_phys_rm VALUES (1, 100)")) + .parse_statement())); + static_cast(exec.execute(*Parser(std::make_unique("COMMIT")).parse_statement())); + + // UPDATE then ROLLBACK with fault injection + static_cast(exec.execute(*Parser(std::make_unique("BEGIN")).parse_statement())); + static_cast( + exec.execute(*Parser(std::make_unique("UPDATE upd_phys_rm SET val = 999 WHERE id = 1")) + .parse_statement())); + + // Arm fault for physical_remove failure during UPDATE undo + cloudsql::common::FaultInjection::instance().set_fault(cloudsql::common::FAULT_PHYSICAL_REMOVE); + static_cast(exec.execute(*Parser(std::make_unique("ROLLBACK")).parse_statement())); + cloudsql::common::FaultInjection::instance().clear(); + + static_cast(std::remove("./test_data/upd_phys_rm.heap")); + static_cast(std::remove("./test_data/upd_phys_rm.idx")); +} + +TEST(TransactionManagerTests, UndoUpdateUndoRemoveFailure) { + // Test FAULT_UNDO_REMOVE branch in UPDATE undo old_rid restore + storage::StorageManager disk_manager("./test_data"); + disk_manager.create_dir_if_not_exists(); + recovery::LogManager log_mgr("./test_data/upd_undo_rm_fault.dat"); + storage::BufferPoolManager bpm(cloudsql::config::Config::DEFAULT_BUFFER_POOL_SIZE, disk_manager, + &log_mgr); + auto catalog = Catalog::create(); + LockManager lm; + TransactionManager tm(lm, *catalog, bpm, &log_mgr); + executor::QueryExecutor exec(*catalog, bpm, lm, tm); + + static_cast(std::remove("./test_data/upd_undo_rm.heap")); + static_cast(std::remove("./test_data/upd_undo_rm.idx")); + + static_cast( + exec.execute(*Parser(std::make_unique("CREATE TABLE upd_undo_rm (id INT, val INT)")) + .parse_statement())); + static_cast( + exec.execute(*Parser(std::make_unique("CREATE INDEX idx_uur ON upd_undo_rm (val)")) + .parse_statement())); + static_cast( + exec.execute(*Parser(std::make_unique("INSERT INTO upd_undo_rm VALUES (1, 100)")) + .parse_statement())); + static_cast(exec.execute(*Parser(std::make_unique("COMMIT")).parse_statement())); + + // UPDATE then ROLLBACK with fault injection for undo_remove + static_cast(exec.execute(*Parser(std::make_unique("BEGIN")).parse_statement())); + static_cast( + exec.execute(*Parser(std::make_unique("UPDATE upd_undo_rm SET val = 999 WHERE id = 1")) + .parse_statement())); + + // Arm fault for undo_remove failure during UPDATE undo (old_rid restore) + cloudsql::common::FaultInjection::instance().set_fault(cloudsql::common::FAULT_UNDO_REMOVE); + static_cast(exec.execute(*Parser(std::make_unique("ROLLBACK")).parse_statement())); + cloudsql::common::FaultInjection::instance().clear(); + + static_cast(std::remove("./test_data/upd_undo_rm.heap")); + static_cast(std::remove("./test_data/upd_undo_rm.idx")); +} + } // namespace From 0afc68bf3b2b67c4690fc85a692fcd2baff9e2d8 Mon Sep 17 00:00:00 2001 From: poyrazK <83272398+poyrazK@users.noreply.github.com> Date: Thu, 30 Apr 2026 13:14:27 +0000 Subject: [PATCH 2/3] style: automated clang-format fixes --- tests/operator_tests.cpp | 3 ++- tests/query_executor_tests.cpp | 4 ++-- tests/transaction_manager_tests.cpp | 12 ++++++------ 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/operator_tests.cpp b/tests/operator_tests.cpp index 3934dcf..25ce68e 100644 --- a/tests/operator_tests.cpp +++ b/tests/operator_tests.cpp @@ -1263,7 +1263,8 @@ TEST_F(OperatorTests, ProjectNextViewNonSimpleProjection) { auto scan = make_buffer_scan("test_table", data, schema); - // Use a constant expression instead of column reference — this makes is_simple_projection_ = false + // Use a constant expression instead of column reference — this makes is_simple_projection_ = + // false std::vector> cols; cols.push_back(const_expr(common::Value::make_int64(42))); // constant, not column auto project = make_project(std::move(scan), std::move(cols)); diff --git a/tests/query_executor_tests.cpp b/tests/query_executor_tests.cpp index f829930..7797706 100644 --- a/tests/query_executor_tests.cpp +++ b/tests/query_executor_tests.cpp @@ -197,8 +197,8 @@ TEST_F(QueryExecutorTests, InsertBatchModeSkipsLockAcquisition) { execute_sql(env.executor, "BEGIN"); // Multi-row INSERT - should succeed without lock acquisition - const auto res = execute_sql( - env.executor, "INSERT INTO test_table VALUES (1, 10), (2, 20), (3, 30)"); + const auto res = + execute_sql(env.executor, "INSERT INTO test_table VALUES (1, 10), (2, 20), (3, 30)"); EXPECT_TRUE(res.success()); EXPECT_EQ(res.rows_affected(), 3U); diff --git a/tests/transaction_manager_tests.cpp b/tests/transaction_manager_tests.cpp index 44e9064..da0628c 100644 --- a/tests/transaction_manager_tests.cpp +++ b/tests/transaction_manager_tests.cpp @@ -1558,9 +1558,9 @@ TEST(TransactionManagerTests, UndoUpdatePhysicalRemoveFailure) { // UPDATE then ROLLBACK with fault injection static_cast(exec.execute(*Parser(std::make_unique("BEGIN")).parse_statement())); - static_cast( - exec.execute(*Parser(std::make_unique("UPDATE upd_phys_rm SET val = 999 WHERE id = 1")) - .parse_statement())); + static_cast(exec.execute( + *Parser(std::make_unique("UPDATE upd_phys_rm SET val = 999 WHERE id = 1")) + .parse_statement())); // Arm fault for physical_remove failure during UPDATE undo cloudsql::common::FaultInjection::instance().set_fault(cloudsql::common::FAULT_PHYSICAL_REMOVE); @@ -1599,9 +1599,9 @@ TEST(TransactionManagerTests, UndoUpdateUndoRemoveFailure) { // UPDATE then ROLLBACK with fault injection for undo_remove static_cast(exec.execute(*Parser(std::make_unique("BEGIN")).parse_statement())); - static_cast( - exec.execute(*Parser(std::make_unique("UPDATE upd_undo_rm SET val = 999 WHERE id = 1")) - .parse_statement())); + static_cast(exec.execute( + *Parser(std::make_unique("UPDATE upd_undo_rm SET val = 999 WHERE id = 1")) + .parse_statement())); // Arm fault for undo_remove failure during UPDATE undo (old_rid restore) cloudsql::common::FaultInjection::instance().set_fault(cloudsql::common::FAULT_UNDO_REMOVE); From af12d4903d6dd0e8c145536f33586acf12a6bc88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Poyraz=20K=C3=BC=C3=A7=C3=BCkarslan?= <83272398+PoyrazK@users.noreply.github.com> Date: Thu, 30 Apr 2026 18:24:47 +0300 Subject: [PATCH 3/3] fix(txn): add assertions to UPDATE fault-injection tests Replace exec.execute("ROLLBACK") with tm.abort(txn) + EXPECT_EQ in UndoUpdatePhysicalRemoveFailure and UndoUpdateUndoRemoveFailure, matching the pattern fixed in e5e8e73 for the INSERT/DELETE fault tests. Both tests now verify the transaction reaches ABORTED state after rollback with fault injection. --- tests/transaction_manager_tests.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/transaction_manager_tests.cpp b/tests/transaction_manager_tests.cpp index da0628c..421746e 100644 --- a/tests/transaction_manager_tests.cpp +++ b/tests/transaction_manager_tests.cpp @@ -1557,14 +1557,15 @@ TEST(TransactionManagerTests, UndoUpdatePhysicalRemoveFailure) { static_cast(exec.execute(*Parser(std::make_unique("COMMIT")).parse_statement())); // UPDATE then ROLLBACK with fault injection - static_cast(exec.execute(*Parser(std::make_unique("BEGIN")).parse_statement())); + Transaction* txn = tm.begin(); static_cast(exec.execute( *Parser(std::make_unique("UPDATE upd_phys_rm SET val = 999 WHERE id = 1")) .parse_statement())); // Arm fault for physical_remove failure during UPDATE undo cloudsql::common::FaultInjection::instance().set_fault(cloudsql::common::FAULT_PHYSICAL_REMOVE); - static_cast(exec.execute(*Parser(std::make_unique("ROLLBACK")).parse_statement())); + tm.abort(txn); + EXPECT_EQ(txn->get_state(), TransactionState::ABORTED); cloudsql::common::FaultInjection::instance().clear(); static_cast(std::remove("./test_data/upd_phys_rm.heap")); @@ -1598,14 +1599,15 @@ TEST(TransactionManagerTests, UndoUpdateUndoRemoveFailure) { static_cast(exec.execute(*Parser(std::make_unique("COMMIT")).parse_statement())); // UPDATE then ROLLBACK with fault injection for undo_remove - static_cast(exec.execute(*Parser(std::make_unique("BEGIN")).parse_statement())); + Transaction* txn = tm.begin(); static_cast(exec.execute( *Parser(std::make_unique("UPDATE upd_undo_rm SET val = 999 WHERE id = 1")) .parse_statement())); // Arm fault for undo_remove failure during UPDATE undo (old_rid restore) cloudsql::common::FaultInjection::instance().set_fault(cloudsql::common::FAULT_UNDO_REMOVE); - static_cast(exec.execute(*Parser(std::make_unique("ROLLBACK")).parse_statement())); + tm.abort(txn); + EXPECT_EQ(txn->get_state(), TransactionState::ABORTED); cloudsql::common::FaultInjection::instance().clear(); static_cast(std::remove("./test_data/upd_undo_rm.heap"));